✨ Tag Controller CRD, Controller & Webhooks - #1865
Open
aniket-deole wants to merge 1 commit into
Open
Conversation
aniket-deole
force-pushed
the
aniketd/tag-controller-part-one
branch
2 times, most recently
from
August 31, 2026 14:32
1ad11fd to
ab81886
Compare
aniket-deole
marked this pull request as ready for review
August 31, 2026 14:33
aniket-deole
force-pushed
the
aniketd/tag-controller-part-one
branch
2 times, most recently
from
August 31, 2026 15:54
2e1c8e8 to
cba1e09
Compare
Contributor
Minimum allowed line rate is |
aruneshpa
reviewed
Aug 31, 2026
| // both operations are denied, which wedges owner-reference cleanup and | ||
| // namespace termination respectively. | ||
| var allowedSystemAccountsForTag = map[string]struct{}{ | ||
| "system:serviceaccount:kube-system:generic-garbage-collector": {}, |
Collaborator
There was a problem hiding this comment.
Is this defined elsewhere in upstream k8s/k8s so we don't have to have strings here? There's also a worry of these service accounts changing which may break this code.
Contributor
Author
There was a problem hiding this comment.
We have a bunch of similar strings defined in webhooks/persistentvolumeclaim/validation/persistentvolumeclaim_validator.go
Comment on lines
+175
to
+190
| // validateLabelKey requires spec.key to be non-empty and a valid | ||
| // Kubernetes label key. | ||
| func validateLabelKey(key string) field.ErrorList { | ||
| return unversionedvalidation.ValidateLabelName(key, specKeyPath) | ||
| } | ||
|
|
||
| // validateLabelValue requires spec.value to be a valid Kubernetes label | ||
| // value. An empty value is permitted. | ||
| func validateLabelValue(value string) field.ErrorList { | ||
| var fieldErrs field.ErrorList | ||
| for _, msg := range utilvalidation.IsValidLabelValue(value) { | ||
| fieldErrs = append(fieldErrs, field.Invalid(specValuePath, value, msg)) | ||
| } | ||
|
|
||
| return fieldErrs | ||
| } |
Collaborator
There was a problem hiding this comment.
Why not use https://pkg.go.dev/k8s.io/apimachinery@v0.37.0/pkg/apis/meta/v1/validation#ValidateLabels?
That seems to combine both of these?
Contributor
Author
There was a problem hiding this comment.
There is separate validation as the spec contains spec.key & spec.value.
The fieldPath argument accepts a single path.
| } | ||
| }() | ||
|
|
||
| return r.ReconcileNormal(ctx, &obj) |
Collaborator
There was a problem hiding this comment.
Hmm. Shouldn't we handle Tags that are marked for deletion (e.g., a non-zero DeletionTimestamp)?
Contributor
Author
There was a problem hiding this comment.
Tags are garbage collected. Do you think we still need explicit deletion?
aniket-deole
force-pushed
the
aniketd/tag-controller-part-one
branch
from
September 1, 2026 05:05
cba1e09 to
416f92b
Compare
aniket-deole
force-pushed
the
aniketd/tag-controller-part-one
branch
from
September 1, 2026 19:47
416f92b to
8e1f341
Compare
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
2 participants
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.
What does this PR do, and why is it needed?
Add Tag CRD, controller, and webhook for VM tagging
Add a new Tag CRD (vsphere.policy.vmware.com/v1alpha1) with a
controller and a validating webhook. A Tag stands for one vCenter
tag (a key/value pair) used for VM affinity. It is owned by one or
more VirtualMachines. The controller marks a Tag Ready, copies its
key/value onto labels, and deletes the Tag once all owners are gone.
Which issue(s) is/are addressed by this PR? (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes # vmop-3882
Are there any special notes for your reviewer: NA
Please add a release note if necessary: