Skip to content

The Big LXC-Iptag Update #3531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

DesertGamer
Copy link

@DesertGamer DesertGamer commented Mar 31, 2025

feat: Major update of IP-Tag script

  • Renamed script to add-iptag.sh
  • Added support for VM IP tagging
  • Added service existence check
  • Added configuration migration from old path
  • Added update functionality for existing installations
  • Fixed status check interval parameter
  • Improved error handling
  • Updated installation path structure

✍️ Description

🔗 Related PR / Issue

Link: #

✅ Prerequisites (X in brackets)

  • Self-review completed – Code follows project standards.
  • Tested thoroughly – Changes work as expected.
  • No breaking changes – Existing functionality remains intact.
  • No security risks – No hardcoded secrets, unnecessary privilege escalations, or permission issues.

🛠️ Type of Change (X in brackets)

  • 🐞 Bug fix – Resolves an issue without breaking functionality.
  • New feature – Adds new, non-breaking functionality.
  • 💥 Breaking change – Alters existing functionality in a way that may require updates.
  • 🆕 New script – A fully functional and tested script or script set.
  • 🌍 Website update – Changes to website-related JSON files or metadata.
  • 🔧 Refactoring / Code Cleanup – Improves readability or maintainability without changing functionality.
  • 📝 Documentation update – Changes to README, AppName.md, CONTRIBUTING.md, or other docs.

🔍 Code & Security Review (X in brackets)

  • Follows Code_Audit.md & CONTRIBUTING.md guidelines

📋 Additional Information (optional)

feat: Major update of IP-Tag script

- Renamed script to add-iptag.sh
- Added support for VM IP tagging
- Added service existence check
- Added configuration migration from old path
- Added update functionality for existing installations
- Fixed status check interval parameter
- Improved error handling
- Updated installation path structure
@DesertGamer DesertGamer requested a review from a team as a code owner March 31, 2025 13:44
@github-actions github-actions bot added the new script A change that adds a new script label Mar 31, 2025
@DesertGamer DesertGamer changed the title Create add-iptag.sh The Big LXC-Iptag Update Mar 31, 2025
@MickLesk
Copy link
Member

MickLesk commented Mar 31, 2025

puh heavy script. thx. can you put this pr into draft and create the file into https://github.com/community-scripts/ProxmoxVED Repo too?

@DesertGamer
Copy link
Author

puh heavy script. thx. can you put this pr into draft and create the file into https://github.com/community-scripts/ProxmoxVED Repo too?

I don't quite understand where to upload this file, can you tell me?

refactor(script): optimize code structure and reduce size

- Consolidate status check functions into single check_status_changed
- Unify tag update logic into single update_tags function
- Optimize IP validation and CIDR checking
- Remove duplicate code and redundant functions
- Reduce system calls and improve performance
- Simplify conditional logic

BREAKING CHANGE: Function names and parameters have been changed
@DesertGamer
Copy link
Author

I noticed that I uploaded the wrong version of the project, now I've uploaded the optimized one.

@MickLesk
Copy link
Member

https://github.com/community-scripts/ProxmoxVED/pulls here, its the dev repo for testing

@MickLesk MickLesk marked this pull request as draft April 1, 2025 14:39
- Reduce code size and complexity
- Add informative status messages
- Improve tag handling and IP validation
- Enhance error reporting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new script A change that adds a new script
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants