Repository navigation
feat(api): add tags to Instance, VPC, Network and SecurityGroup specs - #74
Merged
Merged
Conversation
Add an optional spec.tags string list to the Instance, VPC, Network and SecurityGroup specs so the control plane can persist user-defined resource tags on the CR and read them back through the REST API and UI. Without the field, strict field validation rejects spec.tags writes, which blocks core#256 (VM tags) and the upcoming network/VPC tagging.
Matheus-Thurler
left a comment
Contributor
There was a problem hiding this comment.
Thanks, the change is small and correct. One request before merge:
- Bound the field.
spec.tagshas no limits, so any client can write an unbounded list to etcd. Please add markers, for example+kubebuilder:validation:MaxItems=50and+kubebuilder:validation:items:MaxLength=63, on all four specs and regenerate.
Follow-ups after merge (not blocking this PR):
- Sync the regenerated CRDs in helm-charts (
make sync-crds-chart). - The installed CRDs carry
helm.sh/resource-policy: keep, so existing clusters need an explicitkubectl apply. - This must land and be applied before core#256 is deployed.
Collaborator
Author
SummaryImplemented the owner's review request from PR #74 to bound the Changes MadeType definitions (4 files):
Deepcopy (1 file):
CRDs (4 files):
Verification
The kubebuilder markers and CRD schema now enforce maximum 50 tags per resource and maximum 63 characters per tag string, preventing unbounded writes to etcd. Changes generated with |
2 of 4 tasks
1 of 2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add an optional
spec.tags([]string) to theInstance,VPC,NetworkandSecurityGroupspecs.The control plane (
virtfoundry/core) persists user-defined resource tags directly on the CR and reads them back through the REST API and UI. The field was missing from the schemas, so on a cluster with these CRDs installed, strict field validation rejects the write:This blocks core#256 (VM tags, already open) and the upcoming network/VPC tagging work.
make manifests generate(config/crd/bases/*,zz_generated.deepcopy.go).Cross-repo follow-up
CRDs are shipped through the
virtfoundry-crdschart in virtfoundry/helm-charts. After this merges and a version is cut, sync the regenerated CRDs there. Note that the installed CRD carrieshelm.sh/resource-policy: keep, so an existing cluster needs an explicitkubectl applyto pick up the new field.Test plan
make testmake lintandmake lint-configmake verify-chart-rbac