diff --git a/cmd/hyperconverged-cluster-operator/main.go b/cmd/hyperconverged-cluster-operator/main.go index a63996f50..864c260d0 100644 --- a/cmd/hyperconverged-cluster-operator/main.go +++ b/cmd/hyperconverged-cluster-operator/main.go @@ -56,6 +56,7 @@ import ( "github.com/kubevirt/hyperconverged-cluster-operator/api" hcov1beta1 "github.com/kubevirt/hyperconverged-cluster-operator/api/v1beta1" "github.com/kubevirt/hyperconverged-cluster-operator/cmd/cmdcommon" + "github.com/kubevirt/hyperconverged-cluster-operator/controllers/admissionpolicy" "github.com/kubevirt/hyperconverged-cluster-operator/controllers/crd" "github.com/kubevirt/hyperconverged-cluster-operator/controllers/descheduler" "github.com/kubevirt/hyperconverged-cluster-operator/controllers/handlers" @@ -254,6 +255,12 @@ func main() { } } + if err = admissionpolicy.RegisterReconciler(mgr); err != nil { + logger.Error(err, "failed to register the admission policy controller") + eventEmitter.EmitEvent(nil, corev1.EventTypeWarning, "InitError", "Unable to register admission policy controller; "+err.Error()) + os.Exit(1) + } + err = createPriorityClass(ctx, mgr) cmdHelper.ExitOnError(err, "Failed creating PriorityClass") @@ -350,6 +357,12 @@ func getCacheOption(operatorNamespace string, ci hcoutil.ClusterInfo) cache.Opti Field: namespaceSelector, }, &apiextensionsv1.CustomResourceDefinition{}: {}, + &admissionregistrationv1.ValidatingAdmissionPolicy{}: { + Label: labels.SelectorFromSet(labels.Set{hcoutil.AppLabel: hcoutil.HyperConvergedName}), + }, + &admissionregistrationv1.ValidatingAdmissionPolicyBinding{}: { + Label: labels.SelectorFromSet(labels.Set{hcoutil.AppLabel: hcoutil.HyperConvergedName}), + }, }, } diff --git a/cmd/hyperconverged-cluster-webhook/main.go b/cmd/hyperconverged-cluster-webhook/main.go index 5d1812986..bb95d32aa 100644 --- a/cmd/hyperconverged-cluster-webhook/main.go +++ b/cmd/hyperconverged-cluster-webhook/main.go @@ -173,7 +173,7 @@ func main() { err = bearertokencontroller.RegisterReconciler(mgr, ci, eventEmitter) cmdHelper.ExitOnError(err, "Cannot register the Bearer Token reconciler") - if err = webhooks.SetupWebhookWithManager(ctx, mgr, ci.IsOpenshift(), hcoTLSSecurityProfile); err != nil { + if err = webhooks.SetupWebhookWithManager(mgr, ci.IsOpenshift(), hcoTLSSecurityProfile); err != nil { logger.Error(err, "unable to create webhook", "webhook", "HyperConverged") eventEmitter.EmitEvent(nil, corev1.EventTypeWarning, "InitError", "Unable to create webhook") os.Exit(1) diff --git a/controllers/admissionpolicy/admission_policy_controller.go b/controllers/admissionpolicy/admission_policy_controller.go new file mode 100644 index 000000000..cbb3c3820 --- /dev/null +++ b/controllers/admissionpolicy/admission_policy_controller.go @@ -0,0 +1,205 @@ +package admissionpolicy + +import ( + "context" + "errors" + "fmt" + "reflect" + + "github.com/go-logr/logr" + "github.com/google/uuid" + operatorhandler "github.com/operator-framework/operator-lib/handler" + admissionv1 "k8s.io/api/admissionregistration/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + k8stypes "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" + + hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util" +) + +const controllerName = "admission-policy-controller" + +var ( + initLogger = logf.Log.WithName(controllerName) + + randomConstSuffix = uuid.New().String() + + startupReq = reconcile.Request{ + NamespacedName: k8stypes.NamespacedName{ + Name: "startup-req-" + randomConstSuffix, + }, + } +) + +// RegisterReconciler creates a new Nodes Reconciler and registers it into manager. +func RegisterReconciler(mgr manager.Manager) error { + startupEvent := make(chan event.GenericEvent, 1) + defer close(startupEvent) + + r := newReconciler(mgr, startupEvent) + + startupEvent <- event.GenericEvent{} + + return add(mgr, r) +} + +// newReconciler returns a new reconcile.Reconciler +func newReconciler(mgr manager.Manager, startupEvent <-chan event.GenericEvent) *ReconcileAdmissionPolicy { + initLogger.Info("Initializing the admission policy controller") + + r := &ReconcileAdmissionPolicy{ + Client: mgr.GetClient(), + startupEvent: startupEvent, + } + + return r +} + +// add adds a new Controller to mgr with r as the reconcile.Reconciler +func add(mgr manager.Manager, r *ReconcileAdmissionPolicy) error { + // Create a new controller + c, err := controller.New(controllerName, mgr, controller.Options{Reconciler: r}) + if err != nil { + return err + } + + // Watch for changes to the ValidatingAdmissionPolicy + if err = c.Watch( + source.Kind[*admissionv1.ValidatingAdmissionPolicy]( + mgr.GetCache(), &admissionv1.ValidatingAdmissionPolicy{}, + &operatorhandler.InstrumentedEnqueueRequestForObject[*admissionv1.ValidatingAdmissionPolicy]{}, + policyPredicate, + ), + ); err != nil { + return err + } + + // Watch for changes to the ValidatingAdmissionPolicyBinding + if err = c.Watch( + source.Kind[*admissionv1.ValidatingAdmissionPolicyBinding]( + mgr.GetCache(), &admissionv1.ValidatingAdmissionPolicyBinding{}, + &handler.TypedEnqueueRequestForObject[*admissionv1.ValidatingAdmissionPolicyBinding]{}, + bindingPredicate, + ), + ); err != nil { + return err + } + + return c.Watch( + source.Channel( + r.startupEvent, + handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []reconcile.Request { + logr.FromContextOrDiscard(ctx).Info("first reconciliation of ValidatingAdmissionPolicy") + return []reconcile.Request{startupReq} + }), + ), + ) +} + +// ReconcileAdmissionPolicy reconciles the ValidatingAdmissionPolicy and ValidatingAdmissionPolicyBinding +type ReconcileAdmissionPolicy struct { + client.Client + startupEvent <-chan event.GenericEvent +} + +// Reconcile updates the ValidatingAdmissionPolicy and ValidatingAdmissionPolicyBinding +func (r *ReconcileAdmissionPolicy) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { + logger, err := logr.FromContext(ctx) + if err != nil { + logger = initLogger.WithValues("Request.Name", req.Name) + } + + logger.Info(fmt.Sprintf("Reconciling admission policy %s", req.Name)) + + startup := startupReq == req + var policyErr, bindingErr error + + if req.Name == policyName || startup { + policyErr = r.reconcilePolicy(ctx, logger) + } + + if req.Name == policyBindingName || startup { + bindingErr = r.reconcileBinding(ctx, logger) + } + + err = errors.Join(policyErr, bindingErr) + if err != nil { + logger.Error(err, "Reconciliation failed") + } + + return reconcile.Result{}, err +} + +func (r *ReconcileAdmissionPolicy) reconcilePolicy(ctx context.Context, logger logr.Logger) error { + policy := getRequiredPolicy() + key := client.ObjectKeyFromObject(policy) + foundPolicy := &admissionv1.ValidatingAdmissionPolicy{} + + if err := r.Get(ctx, key, foundPolicy); err != nil { + if k8serrors.IsNotFound(err) { + logger.Info("ValidatingAdmissionPolicy does not exist; creating it", "name", policy.Name) + return r.Create(ctx, policy.DeepCopy()) + } + + return err + } + + changed := false + if !reflect.DeepEqual(foundPolicy.Spec, policy.Spec) { + policy.Spec.DeepCopyInto(&foundPolicy.Spec) + changed = true + } + + if !hcoutil.CompareLabels(policy, foundPolicy) { + hcoutil.MergeLabels(&policy.ObjectMeta, &foundPolicy.ObjectMeta) + changed = true + } + + if changed { + logger.Info("ValidatingAdmissionPolicy was modified; updating it", "name", policy.Name) + return r.Update(ctx, foundPolicy) + } + + return nil +} + +func (r *ReconcileAdmissionPolicy) reconcileBinding(ctx context.Context, logger logr.Logger) error { + binding := getRequiredBinding() + + key := client.ObjectKeyFromObject(binding) + foundBinding := &admissionv1.ValidatingAdmissionPolicyBinding{} + + if err := r.Get(ctx, key, foundBinding); err != nil { + if k8serrors.IsNotFound(err) { + logger.Info("ValidatingAdmissionPolicyBinding does not exist; creating it", "name", binding.Name) + return r.Create(ctx, binding.DeepCopy()) + } + + return err + } + + changed := false + if !reflect.DeepEqual(foundBinding.Spec, binding.Spec) { + binding.Spec.DeepCopyInto(&foundBinding.Spec) + changed = true + } + + if !hcoutil.CompareLabels(binding, foundBinding) { + hcoutil.MergeLabels(&binding.ObjectMeta, &foundBinding.ObjectMeta) + changed = true + } + + if changed { + logger.Info("ValidatingAdmissionPolicyBinding was modified; updating it", "name", binding.Name) + return r.Update(ctx, foundBinding) + } + + return nil +} diff --git a/controllers/admissionpolicy/admission_policycontroller_test.go b/controllers/admissionpolicy/admission_policycontroller_test.go new file mode 100644 index 000000000..71c2e9632 --- /dev/null +++ b/controllers/admissionpolicy/admission_policycontroller_test.go @@ -0,0 +1,233 @@ +package admissionpolicy + +import ( + "context" + "os" + "sync" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + admissionv1 "k8s.io/api/admissionregistration/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/kubevirt/hyperconverged-cluster-operator/controllers/commontestutils" + "github.com/kubevirt/hyperconverged-cluster-operator/pkg/ownresources" + fakeownresources "github.com/kubevirt/hyperconverged-cluster-operator/pkg/ownresources/fake" + hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util" +) + +func TestAdmissionPolicyController(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "AdmissionPolicy Controller Suite") +} + +var _ = Describe("AdmissionPolicyController", func() { + + BeforeEach(func() { + origNS, hasNSVar := os.LookupEnv(hcoutil.OperatorNamespaceEnv) + Expect(os.Setenv(hcoutil.OperatorNamespaceEnv, commontestutils.Namespace)).To(Succeed()) + + fakeownresources.OLMV0OwnResourcesMock() + resetOnces() + + DeferCleanup(func() { + if hasNSVar { + Expect(os.Setenv(hcoutil.OperatorNamespaceEnv, origNS)).To(Succeed()) + } else { + Expect(os.Unsetenv(hcoutil.OperatorNamespaceEnv)).To(Succeed()) + } + + fakeownresources.ResetOwnResources() + resetOnces() + }) + }) + + Context("test creation", func() { + It("Should create a new AdmissionPolicy and binding on startup", func(ctx context.Context) { + var resources []client.Object + cli := commontestutils.InitClient(resources) + + r := &ReconcileAdmissionPolicy{ + Client: cli, + } + res, err := r.Reconcile(ctx, startupReq) + Expect(err).ToNot(HaveOccurred()) + Expect(res.IsZero()).To(BeTrue()) + + policy := getRequiredPolicy() + foundPolicy := &admissionv1.ValidatingAdmissionPolicy{} + Expect(cli.Get(ctx, client.ObjectKeyFromObject(policy), foundPolicy)).To(Succeed()) + Expect(foundPolicy.Spec).To(Equal(policy.Spec)) + + binding := getRequiredBinding() + foundBinding := &admissionv1.ValidatingAdmissionPolicyBinding{} + Expect(cli.Get(ctx, client.ObjectKeyFromObject(binding), foundBinding)).To(Succeed()) + Expect(foundBinding.Spec).To(Equal(binding.Spec)) + }) + + It("Should only create a new AdmissionPolicy if it's missing", func(ctx context.Context) { + var resources []client.Object + cli := commontestutils.InitClient(resources) + + r := &ReconcileAdmissionPolicy{ + Client: cli, + } + + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: policyName, + }, + } + + res, err := r.Reconcile(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.IsZero()).To(BeTrue()) + + policy := getRequiredPolicy() + foundPolicy := &admissionv1.ValidatingAdmissionPolicy{} + Expect(cli.Get(ctx, client.ObjectKeyFromObject(policy), foundPolicy)).To(Succeed()) + Expect(foundPolicy.Spec).To(Equal(policy.Spec)) + + binding := getRequiredBinding() + foundBinding := &admissionv1.ValidatingAdmissionPolicyBinding{} + Expect(cli.Get(ctx, client.ObjectKeyFromObject(binding), foundBinding)).To(MatchError(k8serrors.IsNotFound, "expected to not be found")) + }) + + It("Should only create a new Binding if it's missing", func(ctx context.Context) { + var resources []client.Object + cli := commontestutils.InitClient(resources) + + r := &ReconcileAdmissionPolicy{ + Client: cli, + } + + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: policyBindingName, + }, + } + + res, err := r.Reconcile(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.IsZero()).To(BeTrue()) + + policy := getRequiredPolicy() + foundPolicy := &admissionv1.ValidatingAdmissionPolicy{} + Expect(cli.Get(ctx, client.ObjectKeyFromObject(policy), foundPolicy)).To(MatchError(k8serrors.IsNotFound, "expected to not be found")) + + binding := getRequiredBinding() + foundBinding := &admissionv1.ValidatingAdmissionPolicyBinding{} + Expect(cli.Get(ctx, client.ObjectKeyFromObject(binding), foundBinding)).To(Succeed()) + Expect(foundBinding.Spec).To(Equal(binding.Spec)) + }) + }) + + Context("test update", func() { + var ( + cli client.Client + ) + + const ( + wrongExpr = "wrong expression" + wrongPolicyName = "wrongPolicyName" + ) + + BeforeEach(func() { + modifiedPolicy := getRequiredPolicy() + modifiedPolicy.Spec.Validations[0].Expression = wrongExpr + + modifiedPolicyBinding := getRequiredBinding() + modifiedPolicyBinding.Spec.PolicyName = wrongPolicyName + + resources := []client.Object{modifiedPolicy, modifiedPolicyBinding} + + cli = commontestutils.InitClient(resources) + }) + + It("Should update the AdmissionPolicy and the binding on startup", func(ctx context.Context) { + r := &ReconcileAdmissionPolicy{ + Client: cli, + } + + res, err := r.Reconcile(ctx, startupReq) + Expect(err).ToNot(HaveOccurred()) + Expect(res.IsZero()).To(BeTrue()) + + policy := getRequiredPolicy() + foundPolicy := &admissionv1.ValidatingAdmissionPolicy{} + Expect(cli.Get(ctx, client.ObjectKeyFromObject(policy), foundPolicy)).To(Succeed()) + Expect(foundPolicy.Spec).To(Equal(policy.Spec)) + Expect(foundPolicy.OwnerReferences).To(HaveLen(1)) + Expect(foundPolicy.OwnerReferences[0]).To(Equal(ownresources.GetDeploymentRef())) + + binding := getRequiredBinding() + foundBinding := &admissionv1.ValidatingAdmissionPolicyBinding{} + Expect(cli.Get(ctx, client.ObjectKeyFromObject(binding), foundBinding)).To(Succeed()) + Expect(foundBinding.Spec).To(Equal(binding.Spec)) + Expect(foundBinding.OwnerReferences).To(HaveLen(1)) + Expect(foundBinding.OwnerReferences[0]).To(Equal(ownresources.GetDeploymentRef())) + }) + + It("Should only update the AdmissionPolicy if was modified", func(ctx context.Context) { + r := &ReconcileAdmissionPolicy{ + Client: cli, + } + + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: policyName, + }, + } + + res, err := r.Reconcile(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.IsZero()).To(BeTrue()) + + policy := getRequiredPolicy() + foundPolicy := &admissionv1.ValidatingAdmissionPolicy{} + Expect(cli.Get(ctx, client.ObjectKeyFromObject(policy), foundPolicy)).To(Succeed()) + Expect(foundPolicy.Spec).To(Equal(policy.Spec)) + + binding := getRequiredBinding() + foundBinding := &admissionv1.ValidatingAdmissionPolicyBinding{} + Expect(cli.Get(ctx, client.ObjectKeyFromObject(binding), foundBinding)).To(Succeed()) + Expect(foundBinding.Spec.PolicyName).To(Equal(wrongPolicyName)) + }) + + It("Should only update the Binding if it was changed", func(ctx context.Context) { + r := &ReconcileAdmissionPolicy{ + Client: cli, + } + + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: policyBindingName, + }, + } + + res, err := r.Reconcile(ctx, req) + Expect(err).ToNot(HaveOccurred()) + Expect(res.IsZero()).To(BeTrue()) + + policy := getRequiredPolicy() + foundPolicy := &admissionv1.ValidatingAdmissionPolicy{} + Expect(cli.Get(ctx, client.ObjectKeyFromObject(policy), foundPolicy)).To(Succeed()) + Expect(foundPolicy.Spec.Validations[0].Expression).To(Equal(wrongExpr)) + + binding := getRequiredBinding() + foundBinding := &admissionv1.ValidatingAdmissionPolicyBinding{} + Expect(cli.Get(ctx, client.ObjectKeyFromObject(binding), foundBinding)).To(Succeed()) + Expect(foundBinding.Spec).To(Equal(binding.Spec)) + }) + + }) +}) + +func resetOnces() { + policyOnce = &sync.Once{} + bindingOnce = &sync.Once{} +} diff --git a/controllers/admissionpolicy/resources.go b/controllers/admissionpolicy/resources.go new file mode 100644 index 000000000..b2f6dc8fb --- /dev/null +++ b/controllers/admissionpolicy/resources.go @@ -0,0 +1,96 @@ +package admissionpolicy + +import ( + "fmt" + "sync" + + admissionv1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + hcov1beta1 "github.com/kubevirt/hyperconverged-cluster-operator/api/v1beta1" + "github.com/kubevirt/hyperconverged-cluster-operator/pkg/ownresources" + hcoutil "github.com/kubevirt/hyperconverged-cluster-operator/pkg/util" +) + +const ( + policyName = "hyperconverged-namespace-policy" + policyBindingName = policyName + "-binding" +) + +var ( + requiredPolicy *admissionv1.ValidatingAdmissionPolicy + requiredBinding *admissionv1.ValidatingAdmissionPolicyBinding + + policyOnce = &sync.Once{} + bindingOnce = &sync.Once{} + + policyPredicate = predicate.NewTypedPredicateFuncs[*admissionv1.ValidatingAdmissionPolicy](func(policy *admissionv1.ValidatingAdmissionPolicy) bool { + return policy.Name == policyName && policy.DeletionTimestamp == nil + }) + + bindingPredicate = predicate.NewTypedPredicateFuncs[*admissionv1.ValidatingAdmissionPolicyBinding](func(binding *admissionv1.ValidatingAdmissionPolicyBinding) bool { + return binding.Name == policyBindingName && binding.DeletionTimestamp == nil + }) +) + +func getRequiredPolicy() *admissionv1.ValidatingAdmissionPolicy { + policyOnce.Do(func() { + namespace := hcoutil.GetOperatorNamespaceFromEnv() + requiredPolicy = &admissionv1.ValidatingAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: policyName, + Labels: hcoutil.GetLabels(hcov1beta1.HyperConvergedName, hcoutil.AppComponentDeployment), + OwnerReferences: []metav1.OwnerReference{ownresources.GetDeploymentRef()}, + }, + Spec: admissionv1.ValidatingAdmissionPolicySpec{ + FailurePolicy: ptr.To(admissionv1.Fail), + MatchConstraints: &admissionv1.MatchResources{ + MatchPolicy: ptr.To(admissionv1.Equivalent), + NamespaceSelector: &metav1.LabelSelector{}, + ObjectSelector: &metav1.LabelSelector{}, + ResourceRules: []admissionv1.NamedRuleWithOperations{ + { + RuleWithOperations: admissionv1.RuleWithOperations{ + Rule: admissionv1.Rule{ + APIGroups: []string{hcov1beta1.APIVersionGroup}, + APIVersions: []string{hcov1beta1.APIVersionBeta}, + Resources: []string{"hyperconvergeds"}, + Scope: ptr.To(admissionv1.NamespacedScope), + }, + Operations: []admissionv1.OperationType{admissionv1.Create}, + }, + }, + }, + }, + Validations: []admissionv1.Validation{ + { + Expression: fmt.Sprintf(`request.namespace == '%s'`, namespace), + Message: fmt.Sprintf(`HyperConverged CR can only be created in the '%s' namespace.`, namespace), + }, + }, + }, + } + }) + + return requiredPolicy.DeepCopy() +} + +func getRequiredBinding() *admissionv1.ValidatingAdmissionPolicyBinding { + bindingOnce.Do(func() { + requiredBinding = &admissionv1.ValidatingAdmissionPolicyBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: policyBindingName, + Labels: hcoutil.GetLabels(hcov1beta1.HyperConvergedName, hcoutil.AppComponentDeployment), + OwnerReferences: []metav1.OwnerReference{ownresources.GetDeploymentRef()}, + }, + Spec: admissionv1.ValidatingAdmissionPolicyBindingSpec{ + PolicyName: policyName, + ValidationActions: []admissionv1.ValidationAction{admissionv1.Deny}, + }, + } + }) + + return requiredBinding.DeepCopy() +} diff --git a/deploy/cluster_role.yaml b/deploy/cluster_role.yaml index 06961db77..12b2ac65b 100644 --- a/deploy/cluster_role.yaml +++ b/deploy/cluster_role.yaml @@ -1197,6 +1197,18 @@ rules: - create - update - delete +- apiGroups: + - admissionregistration.k8s.io + resources: + - validatingadmissionpolicies + - validatingadmissionpolicybindings + verbs: + - get + - list + - watch + - create + - update + - delete --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/deploy/index-image/community-kubevirt-hyperconverged/1.17.0/manifests/kubevirt-hyperconverged-operator.v1.17.0.clusterserviceversion.yaml b/deploy/index-image/community-kubevirt-hyperconverged/1.17.0/manifests/kubevirt-hyperconverged-operator.v1.17.0.clusterserviceversion.yaml index 27450648c..ede7ef264 100644 --- a/deploy/index-image/community-kubevirt-hyperconverged/1.17.0/manifests/kubevirt-hyperconverged-operator.v1.17.0.clusterserviceversion.yaml +++ b/deploy/index-image/community-kubevirt-hyperconverged/1.17.0/manifests/kubevirt-hyperconverged-operator.v1.17.0.clusterserviceversion.yaml @@ -630,6 +630,18 @@ spec: - create - update - delete + - apiGroups: + - admissionregistration.k8s.io + resources: + - validatingadmissionpolicies + - validatingadmissionpolicybindings + verbs: + - get + - list + - watch + - create + - update + - delete serviceAccountName: hyperconverged-cluster-operator - rules: [] serviceAccountName: hyperconverged-cluster-cli-download diff --git a/deploy/olm-catalog/community-kubevirt-hyperconverged/1.17.0/manifests/kubevirt-hyperconverged-operator.v1.17.0.clusterserviceversion.yaml b/deploy/olm-catalog/community-kubevirt-hyperconverged/1.17.0/manifests/kubevirt-hyperconverged-operator.v1.17.0.clusterserviceversion.yaml index 7a888f350..f15fd4439 100644 --- a/deploy/olm-catalog/community-kubevirt-hyperconverged/1.17.0/manifests/kubevirt-hyperconverged-operator.v1.17.0.clusterserviceversion.yaml +++ b/deploy/olm-catalog/community-kubevirt-hyperconverged/1.17.0/manifests/kubevirt-hyperconverged-operator.v1.17.0.clusterserviceversion.yaml @@ -9,7 +9,7 @@ metadata: certified: "false" console.openshift.io/disable-operand-delete: "true" containerImage: quay.io/kubevirt/hyperconverged-cluster-operator:1.17.0-unstable - createdAt: "2025-12-05 05:06:42" + createdAt: "2025-12-07 16:50:19" description: A unified operator deploying and controlling KubeVirt and its supporting operators with opinionated defaults features.operators.openshift.io/cnf: "false" @@ -630,6 +630,18 @@ spec: - create - update - delete + - apiGroups: + - admissionregistration.k8s.io + resources: + - validatingadmissionpolicies + - validatingadmissionpolicybindings + verbs: + - get + - list + - watch + - create + - update + - delete serviceAccountName: hyperconverged-cluster-operator - rules: [] serviceAccountName: hyperconverged-cluster-cli-download diff --git a/pkg/components/components.go b/pkg/components/components.go index e2cca7d96..dfa4381a7 100644 --- a/pkg/components/components.go +++ b/pkg/components/components.go @@ -674,6 +674,11 @@ func GetClusterPermissions() []rbacv1.PolicyRule { Resources: stringListToSlice("networkpolicies"), Verbs: stringListToSlice("get", "list", "watch", "create", "update", "delete"), }, + { + APIGroups: stringListToSlice(admissionregistrationv1.GroupName), + Resources: stringListToSlice("validatingadmissionpolicies", "validatingadmissionpolicybindings"), + Verbs: stringListToSlice("get", "list", "watch", "create", "update", "delete"), + }, } } diff --git a/pkg/webhooks/setup.go b/pkg/webhooks/setup.go index 11e6c69ba..673a933a1 100644 --- a/pkg/webhooks/setup.go +++ b/pkg/webhooks/setup.go @@ -1,7 +1,6 @@ package webhooks import ( - "context" "os" openshiftconfigv1 "github.com/openshift/api/config/v1" @@ -10,10 +9,7 @@ import ( "github.com/kubevirt/hyperconverged-cluster-operator/pkg/webhooks/mutator" "github.com/kubevirt/hyperconverged-cluster-operator/pkg/webhooks/validator" - admissionregistrationv1 "k8s.io/api/admissionregistration/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -28,7 +24,7 @@ var ( logger = logf.Log.WithName("webhook-setup") ) -func SetupWebhookWithManager(ctx context.Context, mgr ctrl.Manager, isOpenshift bool, hcoTLSSecurityProfile *openshiftconfigv1.TLSSecurityProfile) error { +func SetupWebhookWithManager(mgr ctrl.Manager, isOpenshift bool, hcoTLSSecurityProfile *openshiftconfigv1.TLSSecurityProfile) error { operatorNsEnv := hcoutil.GetOperatorNamespaceFromEnv() decoder := admission.NewDecoder(mgr.GetScheme()) @@ -37,10 +33,6 @@ func SetupWebhookWithManager(ctx context.Context, mgr ctrl.Manager, isOpenshift nsMutator := mutator.NewNsMutator(mgr.GetClient(), decoder, operatorNsEnv) hyperConvergedMutator := mutator.NewHyperConvergedMutator(mgr.GetClient(), decoder) - if err := allowWatchAllNamespaces(ctx, mgr); err != nil { - return err - } - srv := mgr.GetWebhookServer() srv.Register(hcoutil.HCONSWebhookPath, &webhook.Admission{Handler: nsMutator}) @@ -58,38 +50,3 @@ func GetWebhookCertDir() string { return hcoutil.DefaultWebhookCertDir } - -// The OLM limits the webhook scope to the namespaces that are defined in the OperatorGroup -// by setting namespaceSelector in the ValidatingWebhookConfiguration. We would like our webhook to intercept -// requests from all namespaces, and fail them if they're not in the correct namespace for HCO (for CREATE). -// Luckily the OLM does not watch and reconcile the ValidatingWebhookConfiguration so we can simply reset the -// namespaceSelector -func allowWatchAllNamespaces(ctx context.Context, mgr ctrl.Manager) error { - vwcList := &admissionregistrationv1.ValidatingWebhookConfigurationList{} - err := mgr.GetAPIReader().List(ctx, vwcList, client.MatchingLabels{"olm.webhook-description-generate-name": hcoutil.HcoValidatingWebhook}) - if err != nil { - logger.Error(err, "A validating webhook for the HCO was not found") - return err - } - - for _, vwc := range vwcList.Items { - update := false - - for i, wh := range vwc.Webhooks { - if wh.Name == hcoutil.HcoValidatingWebhook { - vwc.Webhooks[i].NamespaceSelector = &metav1.LabelSelector{MatchLabels: map[string]string{}} - update = true - } - } - - if update { - logger.Info("Removing namespace scope from webhook", "webhook", vwc.Name) - err = mgr.GetClient().Update(ctx, &vwc) - if err != nil { - logger.Error(err, "Failed updating webhook", "webhook", vwc.Name) - return err - } - } - } - return nil -} diff --git a/pkg/webhooks/setup_test.go b/pkg/webhooks/setup_test.go index 2796f8655..8147c33cb 100644 --- a/pkg/webhooks/setup_test.go +++ b/pkg/webhooks/setup_test.go @@ -1,7 +1,6 @@ package webhooks import ( - "context" "os" "path" "strings" @@ -69,7 +68,7 @@ var _ = Describe("Hyperconverged API: Webhook", func() { mgr, err := commontestutils.NewManagerMock(&rest.Config{}, manager.Options{WebhookServer: ws, Scheme: cl.Scheme()}, cl, logger) Expect(err).ToNot(HaveOccurred()) - Expect(SetupWebhookWithManager(context.TODO(), mgr, true, nil)).To(Succeed()) + Expect(SetupWebhookWithManager(mgr, true, nil)).To(Succeed()) }) }) diff --git a/pkg/webhooks/validator/validator.go b/pkg/webhooks/validator/validator.go index 8dcb909b7..72b3bfb38 100644 --- a/pkg/webhooks/validator/validator.go +++ b/pkg/webhooks/validator/validator.go @@ -141,10 +141,6 @@ func (wh *WebhookHandler) ValidateCreate(_ context.Context, dryrun bool, hc *v1b return err } - if hc.Namespace != wh.namespace { - return fmt.Errorf("invalid namespace for v1beta1.HyperConverged - please use the %s namespace", wh.namespace) - } - if err := wh.validateDataImportCronTemplates(hc); err != nil { return err } diff --git a/pkg/webhooks/validator/validator_test.go b/pkg/webhooks/validator/validator_test.go index 2b78b0c21..eb7e99ddc 100644 --- a/pkg/webhooks/validator/validator_test.go +++ b/pkg/webhooks/validator/validator_test.go @@ -172,26 +172,10 @@ var _ = Describe("webhooks validator", func() { Expect(res.Result.Message).To(Equal("unknown operation request \"MALFORMED\"")) }) - It("should correctly handle operation errors", func() { - cr.Namespace = ResourceInvalidNamespace - req := newRequest(admissionv1.Create, cr, v1beta1Codec, false) - - res := wh.Handle(ctx, req) - Expect(res.Allowed).To(BeFalse()) - Expect(res.Result.Code).To(Equal(int32(403))) - Expect(res.Result.Reason).To(Equal(metav1.StatusReasonForbidden)) - Expect(res.Result.Message).To(Equal("invalid namespace for v1beta1.HyperConverged - please use the kubevirt-hyperconverged namespace")) - }) - It("should accept creation of a resource with a valid namespace", func() { Expect(wh.ValidateCreate(ctx, dryRun, cr)).To(Succeed()) }) - It("should reject creation of a resource with an arbitrary namespace", func() { - cr.Namespace = ResourceInvalidNamespace - Expect(wh.ValidateCreate(ctx, dryRun, cr)).ToNot(Succeed()) - }) - DescribeTable("Validate annotations", func(annotations map[string]string, assertion types.GomegaMatcher) { cr.Annotations = annotations Expect(wh.ValidateCreate(ctx, dryRun, cr)).To(assertion) @@ -1866,6 +1850,17 @@ var _ = Describe("webhooks validator", func() { Expect(hcoTLSConfigCache).To(Equal(&initialTLSSecurityProfile)) cr.Spec.TLSSecurityProfile = &modernTLSSecurityProfile cr.Namespace = ResourceInvalidNamespace + + cr.Spec.DataImportCronTemplates = []v1beta1.DataImportCronTemplate{ + { + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + util.DataImportCronEnabledAnnotation: "a-non-boolean-value", + }, + }, + }, + } + Expect(wh.ValidateCreate(ctx, false, cr)).ToNot(Succeed()) Expect(hcoTLSConfigCache).To(Equal(&initialTLSSecurityProfile)) })