-
Notifications
You must be signed in to change notification settings - Fork 60
🌱 Protected annotation-to-condition pattern #1278
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,6 +63,7 @@ func ReconcileStatus( | |
|
|
||
| var errs []error | ||
|
|
||
| errs = append(errs, reconcileStatusAnno2Conditions(vmCtx, k8sClient, vcVM, data)...) | ||
| errs = append(errs, reconcileStatusClass(vmCtx, k8sClient, vcVM, data)...) | ||
| errs = append(errs, reconcileStatusPowerState(vmCtx, k8sClient, vcVM, data)...) | ||
| errs = append(errs, reconcileStatusIdentifiers(vmCtx, k8sClient, vcVM, data)...) | ||
|
|
@@ -91,6 +92,53 @@ func ReconcileStatus( | |
| return apierrorsutil.NewAggregate(errs) | ||
| } | ||
|
|
||
| var anno2ConditionRegex = regexp.MustCompile(`^condition.vmoperator.vmware.com.protected/(.+)?$`) | ||
|
|
||
| // reconcileStatusAnno2Conditions sets conditions on the VM based on | ||
| // annotation values. | ||
| func reconcileStatusAnno2Conditions( | ||
| vmCtx pkgctx.VirtualMachineContext, | ||
| _ ctrlclient.Client, | ||
| _ *object.VirtualMachine, | ||
| _ ReconcileStatusData) []error { //nolint:unparam | ||
|
|
||
| for k, v := range vmCtx.VM.Annotations { | ||
| if anno2ConditionRegex.MatchString(k) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the regex with the capture group when not using that, vs |
||
| var ( | ||
| t string | ||
| s metav1.ConditionStatus | ||
| r string | ||
| m string | ||
| ) | ||
| p := strings.Split(v, ";") | ||
| if len(p) > 0 { | ||
| t = p[0] | ||
| } | ||
| if len(p) > 1 { | ||
| s = metav1.ConditionStatus(p[1]) | ||
| } | ||
| if len(p) > 2 { | ||
| r = p[2] | ||
| } | ||
| if len(p) > 3 { | ||
| m = p[3] | ||
| } | ||
| if t != "" { | ||
| switch s { | ||
| case metav1.ConditionFalse: | ||
| conditions.MarkFalse(vmCtx.VM, t, r, m+"%s", "") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| case metav1.ConditionTrue: | ||
| conditions.MarkTrue(vmCtx.VM, t) | ||
| default: | ||
| conditions.MarkUnknown(vmCtx.VM, t, r, m+"%s", "") | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func reconcileStatusClass( | ||
| vmCtx pkgctx.VirtualMachineContext, | ||
| k8sClient ctrlclient.Client, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1988,6 +1988,52 @@ func (v validator) validateAvailabilityZone( | |
| return allErrs | ||
| } | ||
|
|
||
| // protectedAnnotationRegex matches annotations with keys matching the pattern: | ||
| // ^.+\.protected(/.+)?$ | ||
| // | ||
| // Examples that match: | ||
| // - fu.bar.protected | ||
| // - hello.world.protected/sub-key | ||
| // - vmoperator.vmware.com.protected/reconcile-priority | ||
| // | ||
| // Examples that do NOT match: | ||
| // - protected.fu.bar | ||
| // - hello.world.protected.against/sub-key | ||
| var protectedAnnotationRegex = regexp.MustCompile(`^.+\.protected(/.*)?$`) | ||
|
|
||
| // validateProtectedAnnotations validates that annotations matching the | ||
| // protected annotation pattern can only be modified by privileged users. | ||
| func (v validator) validateProtectedAnnotations(vm, oldVM *vmopv1.VirtualMachine) field.ErrorList { | ||
| var allErrs field.ErrorList | ||
| annotationPath := field.NewPath("metadata", "annotations") | ||
|
|
||
| // Collect all protected annotation keys from both old and new VMs | ||
| protectedKeys := make(map[string]struct{}) | ||
|
|
||
| for k := range vm.Annotations { | ||
| if protectedAnnotationRegex.MatchString(k) { | ||
| protectedKeys[k] = struct{}{} | ||
| } | ||
| } | ||
|
|
||
| for k := range oldVM.Annotations { | ||
| if protectedAnnotationRegex.MatchString(k) { | ||
| protectedKeys[k] = struct{}{} | ||
| } | ||
| } | ||
|
|
||
| // Check if any protected annotations have been modified | ||
| for k := range protectedKeys { | ||
| if vm.Annotations[k] != oldVM.Annotations[k] { | ||
| allErrs = append(allErrs, field.Forbidden( | ||
| annotationPath.Key(k), | ||
| modifyAnnotationNotAllowedForNonAdmin)) | ||
| } | ||
| } | ||
|
|
||
| return allErrs | ||
| } | ||
|
|
||
| func (v validator) validateAnnotation(ctx *pkgctx.WebhookRequestContext, vm, oldVM *vmopv1.VirtualMachine) field.ErrorList { | ||
| var allErrs field.ErrorList | ||
|
|
||
|
|
@@ -2012,9 +2058,7 @@ func (v validator) validateAnnotation(ctx *pkgctx.WebhookRequestContext, vm, old | |
| oldVM = &vmopv1.VirtualMachine{} | ||
| } | ||
|
|
||
| if vm.Annotations[pkgconst.ReconcilePriorityAnnotationKey] != oldVM.Annotations[pkgconst.ReconcilePriorityAnnotationKey] { | ||
| allErrs = append(allErrs, field.Forbidden(annotationPath.Key(pkgconst.ReconcilePriorityAnnotationKey), modifyAnnotationNotAllowedForNonAdmin)) | ||
| } | ||
| allErrs = append(allErrs, v.validateProtectedAnnotations(vm, oldVM)...) | ||
|
|
||
| if vm.Annotations[vmopv1.InstanceIDAnnotation] != oldVM.Annotations[vmopv1.InstanceIDAnnotation] { | ||
| allErrs = append(allErrs, field.Forbidden(annotationPath.Key(vmopv1.InstanceIDAnnotation), modifyAnnotationNotAllowedForNonAdmin)) | ||
|
|
@@ -2036,14 +2080,6 @@ func (v validator) validateAnnotation(ctx *pkgctx.WebhookRequestContext, vm, old | |
| allErrs = append(allErrs, field.Forbidden(annotationPath.Key(vmopv1.ImportedVMAnnotation), modifyAnnotationNotAllowedForNonAdmin)) | ||
| } | ||
|
|
||
| if vm.Annotations[pkgconst.SkipDeletePlatformResourceKey] != oldVM.Annotations[pkgconst.SkipDeletePlatformResourceKey] { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we remove the annotations from the constant file since they are only used in the tests now? |
||
| allErrs = append(allErrs, field.Forbidden(annotationPath.Key(pkgconst.SkipDeletePlatformResourceKey), modifyAnnotationNotAllowedForNonAdmin)) | ||
| } | ||
|
|
||
| if vm.Annotations[pkgconst.ApplyPowerStateTimeAnnotation] != oldVM.Annotations[pkgconst.ApplyPowerStateTimeAnnotation] { | ||
| allErrs = append(allErrs, field.Forbidden(annotationPath.Key(pkgconst.ApplyPowerStateTimeAnnotation), modifyAnnotationNotAllowedForNonAdmin)) | ||
| } | ||
|
|
||
| for k := range anno2extraconfig.AnnotationsToExtraConfigKeys { | ||
| if vm.Annotations[k] != oldVM.Annotations[k] { | ||
| allErrs = append(allErrs, field.Forbidden(annotationPath.Key(k), modifyAnnotationNotAllowedForNonAdmin)) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more of a nit: but do we not have a standard about the ordering of items in a file: consts, vars, types, funcs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it also be in the constant file with the other annotations?