From a90901fc7fa2b61f5378e36c6c1e2b24545b3450 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sun, 28 May 2023 15:05:58 +0200 Subject: [PATCH 01/15] initial work on dynamic objects VWH --- config/webhook/manifests.yaml | 11 ---------- internal/hncconfig/reconciler.go | 35 ++++++++++++++++++++++++++++++++ internal/setup/webhooks.go | 16 +++++++-------- internal/webhooks/webhooks.go | 5 +++++ 4 files changed, 47 insertions(+), 20 deletions(-) diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index bdf360c22..f858a2e14 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -80,17 +80,6 @@ webhooks: path: /validate-objects failurePolicy: Fail name: objects.hnc.x-k8s.io - rules: - - apiGroups: - - '*' - apiVersions: - - '*' - operations: - - CREATE - - UPDATE - - DELETE - resources: - - '*' sideEffects: None - admissionReviewVersions: - v1 diff --git a/internal/hncconfig/reconciler.go b/internal/hncconfig/reconciler.go index 2d9807be6..7fddfffe6 100644 --- a/internal/hncconfig/reconciler.go +++ b/internal/hncconfig/reconciler.go @@ -8,6 +8,7 @@ import ( "time" "github.com/go-logr/logr" + apiadmissionregistrationv1 "k8s.io/api/admissionregistration/v1" apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -27,6 +28,7 @@ import ( "sigs.k8s.io/hierarchical-namespaces/internal/forest" "sigs.k8s.io/hierarchical-namespaces/internal/objects" "sigs.k8s.io/hierarchical-namespaces/internal/stats" + "sigs.k8s.io/hierarchical-namespaces/internal/webhooks" ) // Reconciler is responsible for determining the HNC configuration from the HNCConfiguration CR, @@ -84,6 +86,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, err } + if err := r.syncObjectWebhookConfigs(ctx); err != nil { + return ctrl.Result{}, err + } + // Create or sync corresponding ObjectReconcilers, if needed. syncErr := r.syncObjectReconcilers(ctx, inst) @@ -232,6 +238,35 @@ func (r *Reconciler) writeSingleton(ctx context.Context, inst *api.HNCConfigurat return nil } +func (r *Reconciler) syncObjectWebhookConfigs(ctx context.Context) error { + var rules []apiadmissionregistrationv1.RuleWithOperations + for gr := range r.activeGVKMode { + rule := apiadmissionregistrationv1.RuleWithOperations{} + rule.APIGroups = []string{gr.Group} + rule.Resources = []string{gr.Resource} + rule.APIVersions = []string{"*"} + rule.Operations = []apiadmissionregistrationv1.OperationType{apiadmissionregistrationv1.Create, apiadmissionregistrationv1.Update, apiadmissionregistrationv1.Delete} + rules = append(rules, rule) + } + + vwc := &apiadmissionregistrationv1.ValidatingWebhookConfiguration{} + if err := r.Get(ctx, client.ObjectKey{Name: webhooks.ValidatingWebhookName}, vwc); err != nil { + return err + } + cleanVWC := vwc.DeepCopy() + + for i, wh := range vwc.Webhooks { + if wh.Name == "objects.hnc.x-k8s.io" { + vwc.Webhooks[i].Rules = rules + } + } + + if err := r.Patch(ctx, vwc, client.MergeFrom(cleanVWC)); err != nil { + return err + } + return nil +} + // syncObjectReconcilers creates or syncs ObjectReconcilers. // // For newly added types in the HNC configuration, the method will create corresponding ObjectReconcilers and diff --git a/internal/setup/webhooks.go b/internal/setup/webhooks.go index 93451857c..0a5932170 100644 --- a/internal/setup/webhooks.go +++ b/internal/setup/webhooks.go @@ -16,17 +16,15 @@ import ( "sigs.k8s.io/hierarchical-namespaces/internal/hrq" ns "sigs.k8s.io/hierarchical-namespaces/internal/namespace" // for some reason, by default this gets imported as "namespace*s*" "sigs.k8s.io/hierarchical-namespaces/internal/objects" + "sigs.k8s.io/hierarchical-namespaces/internal/webhooks" ) const ( serviceName = "hnc-webhook-service" - vwhName = "hnc-validating-webhook-configuration" - mwhName = "hnc-mutating-webhook-configuration" - caName = "hnc-ca" - caOrganization = "hnc" - secretName = "hnc-webhook-server-cert" - certDir = "/tmp/k8s-webhook-server/serving-certs" - apiExtCertDir = "/certs" + caName = "hnc-ca" + caOrganization = "hnc" + secretName = "hnc-webhook-server-cert" + certDir = "/tmp/k8s-webhook-server/serving-certs"apiExtCertDir = "/certs" apiExtServiceName = "hnc-resourcelist" apiExtSecretName = "hnc-resourcelist" apiExtName = "v1alpha2.resources.hnc.x-k8s.io" @@ -50,10 +48,10 @@ func ManageCerts(mgr ctrl.Manager, setupFinished chan struct{}, restartOnSecretR IsReady: setupFinished, Webhooks: []cert.WebhookInfo{{ Type: cert.Validating, - Name: vwhName, + Name: webhooks.ValidatingWebhookName, }, { Type: cert.Mutating, - Name: mwhName, + Name: webhooks.MutatingWebhookName, }}, RestartOnSecretRefresh: restartOnSecretRefresh, }) diff --git a/internal/webhooks/webhooks.go b/internal/webhooks/webhooks.go index db02fcd15..a7abf9ca1 100644 --- a/internal/webhooks/webhooks.go +++ b/internal/webhooks/webhooks.go @@ -14,6 +14,11 @@ import ( "sigs.k8s.io/hierarchical-namespaces/internal/config" ) +const ( + ValidatingWebhookName = "hnc-validating-webhook-configuration" + MutatingWebhookName = "hnc-mutating-webhook-configuration" +) + // IsHNCServiceAccount is inspired by isGKServiceAccount from open-policy-agent/gatekeeper. func IsHNCServiceAccount(user *authnv1.UserInfo) bool { if user == nil { From 9f5553a98cc6634aee245d09eb207ff569f0673a Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sun, 28 May 2023 15:43:00 +0200 Subject: [PATCH 02/15] fix webhook config --- config/webhook/manifests.yaml | 2 ++ config/webhook/webhook_patch.yaml | 20 ++++---------------- internal/hncconfig/reconciler.go | 3 +++ internal/objects/validator.go | 2 +- 4 files changed, 10 insertions(+), 17 deletions(-) diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index f858a2e14..65419266c 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -80,6 +80,8 @@ webhooks: path: /validate-objects failurePolicy: Fail name: objects.hnc.x-k8s.io + rules: + - {} sideEffects: None - admissionReviewVersions: - v1 diff --git a/config/webhook/webhook_patch.yaml b/config/webhook/webhook_patch.yaml index b7d9bcb81..b5ae11a5d 100644 --- a/config/webhook/webhook_patch.yaml +++ b/config/webhook/webhook_patch.yaml @@ -19,19 +19,7 @@ webhooks: namespaceSelector: matchLabels: hnc.x-k8s.io/included-namespace: "true" - rules: - # This overwrites the rules specified in the object validator to patch object - # scope of `namespaced` since there's no kubebuilder marker for `scope`. - # There's no way to just patch "scope: Namespaced" to the first rule, since - # `rules` takes a list of rules and we can only patch the entire `rules`. - - apiGroups: - - '*' - apiVersions: - - '*' - operations: - - CREATE - - UPDATE - - DELETE - resources: - - '*' - scope: "Namespaced" + # This deletes the rules specified in the object validator as we reconcile + # the object validator rules from HNCConfiguration. + # FIXME: Should ideally remove field + rules: [] diff --git a/internal/hncconfig/reconciler.go b/internal/hncconfig/reconciler.go index 7fddfffe6..45b2798ca 100644 --- a/internal/hncconfig/reconciler.go +++ b/internal/hncconfig/reconciler.go @@ -239,12 +239,15 @@ func (r *Reconciler) writeSingleton(ctx context.Context, inst *api.HNCConfigurat } func (r *Reconciler) syncObjectWebhookConfigs(ctx context.Context) error { + namespacedScope := apiadmissionregistrationv1.NamespacedScope + var rules []apiadmissionregistrationv1.RuleWithOperations for gr := range r.activeGVKMode { rule := apiadmissionregistrationv1.RuleWithOperations{} rule.APIGroups = []string{gr.Group} rule.Resources = []string{gr.Resource} rule.APIVersions = []string{"*"} + rule.Scope = &namespacedScope rule.Operations = []apiadmissionregistrationv1.OperationType{apiadmissionregistrationv1.Create, apiadmissionregistrationv1.Update, apiadmissionregistrationv1.Delete} rules = append(rules, rule) } diff --git a/internal/objects/validator.go b/internal/objects/validator.go index 9d2407eb1..24e36894f 100644 --- a/internal/objects/validator.go +++ b/internal/objects/validator.go @@ -41,7 +41,7 @@ const ( // file if you want to change the webhook `rules` and better make the rules // here the same as what's in the webhook_patch.yaml. // -// +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-objects,mutating=false,failurePolicy=fail,groups="*",resources="*",sideEffects=None,verbs=create;update;delete,versions="*",name=objects.hnc.x-k8s.io +// +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-objects,mutating=false,failurePolicy=fail,groups=,resources=,sideEffects=None,verbs=,versions=,name=objects.hnc.x-k8s.io type Validator struct { Log logr.Logger From ecb05179dfedb73e337a47cd2f50cc646440b2e7 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sun, 28 May 2023 16:07:34 +0200 Subject: [PATCH 03/15] fix immediate test issues --- internal/hncconfig/reconciler.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/hncconfig/reconciler.go b/internal/hncconfig/reconciler.go index 45b2798ca..dfea8d6a6 100644 --- a/internal/hncconfig/reconciler.go +++ b/internal/hncconfig/reconciler.go @@ -254,6 +254,11 @@ func (r *Reconciler) syncObjectWebhookConfigs(ctx context.Context) error { vwc := &apiadmissionregistrationv1.ValidatingWebhookConfiguration{} if err := r.Get(ctx, client.ObjectKey{Name: webhooks.ValidatingWebhookName}, vwc); err != nil { + if errors.IsNotFound(err) { + // todo(erikgb): See if the tests can/should be bootstrapped with this webhook + // Webhook not found; nothing to reconcile + return nil + } return err } cleanVWC := vwc.DeepCopy() @@ -261,12 +266,12 @@ func (r *Reconciler) syncObjectWebhookConfigs(ctx context.Context) error { for i, wh := range vwc.Webhooks { if wh.Name == "objects.hnc.x-k8s.io" { vwc.Webhooks[i].Rules = rules + if err := r.Patch(ctx, vwc, client.MergeFrom(cleanVWC)); err != nil { + return err + } + break } } - - if err := r.Patch(ctx, vwc, client.MergeFrom(cleanVWC)); err != nil { - return err - } return nil } From e07e0fd4210c0aa85c86c772e83aeb5abe5b8b9e Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sun, 28 May 2023 18:31:04 +0200 Subject: [PATCH 04/15] configure webhooks in tests (almost) --- config/webhook/kustomization.yaml | 3 --- config/webhook/manifests.yaml | 12 --------- config/webhook/webhook_patch.yaml | 25 ------------------ internal/hncconfig/reconciler.go | 43 ++++++++++++++++++++++++++++--- internal/integtest/setup.go | 9 +++++++ internal/objects/validator.go | 11 -------- internal/setup/reconcilers.go | 2 +- internal/setup/webhooks.go | 4 +-- 8 files changed, 51 insertions(+), 58 deletions(-) delete mode 100644 config/webhook/webhook_patch.yaml diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml index eb3ccebfe..9cf26134e 100644 --- a/config/webhook/kustomization.yaml +++ b/config/webhook/kustomization.yaml @@ -4,6 +4,3 @@ resources: configurations: - kustomizeconfig.yaml - -patchesStrategicMerge: -- webhook_patch.yaml diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 65419266c..b2cb0193c 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -71,18 +71,6 @@ webhooks: resources: - hierarchyconfigurations sideEffects: None -- admissionReviewVersions: - - v1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /validate-objects - failurePolicy: Fail - name: objects.hnc.x-k8s.io - rules: - - {} - sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/config/webhook/webhook_patch.yaml b/config/webhook/webhook_patch.yaml deleted file mode 100644 index b5ae11a5d..000000000 --- a/config/webhook/webhook_patch.yaml +++ /dev/null @@ -1,25 +0,0 @@ ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: ValidatingWebhookConfiguration -metadata: - name: validating-webhook-configuration -webhooks: -- name: objects.hnc.x-k8s.io - timeoutSeconds: 2 - # We only apply this object validator on non-excluded namespaces, which have - # the "included-namespace" label set by the HC reconciler, so that when HNC - # (webhook service specifically) is down, operations in the excluded - # namespaces won't be affected. Validators on HNC CRs are not filtered because - # they are supposed to prevent abuse of HNC CRs in excluded namespaces. - # Namespace validator is not filtered to prevent abuse of the included-namespace - # label on excluded namespaces. Unfortunately, this means that when HNC is - # down, we will block updates on all namespaces, even "excluded" ones, but - # anyone who can update namespaces like `kube-system` should likely be able to - # delete the VWHConfiguration to make the updates. - namespaceSelector: - matchLabels: - hnc.x-k8s.io/included-namespace: "true" - # This deletes the rules specified in the object validator as we reconcile - # the object validator rules from HNCConfiguration. - # FIXME: Should ideally remove field - rules: [] diff --git a/internal/hncconfig/reconciler.go b/internal/hncconfig/reconciler.go index dfea8d6a6..9493bc777 100644 --- a/internal/hncconfig/reconciler.go +++ b/internal/hncconfig/reconciler.go @@ -15,6 +15,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" @@ -24,6 +25,7 @@ import ( api "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2" "sigs.k8s.io/hierarchical-namespaces/internal/apimeta" + "sigs.k8s.io/hierarchical-namespaces/internal/config" "sigs.k8s.io/hierarchical-namespaces/internal/crd" "sigs.k8s.io/hierarchical-namespaces/internal/forest" "sigs.k8s.io/hierarchical-namespaces/internal/objects" @@ -263,16 +265,49 @@ func (r *Reconciler) syncObjectWebhookConfigs(ctx context.Context) error { } cleanVWC := vwc.DeepCopy() + webhookFound := false for i, wh := range vwc.Webhooks { if wh.Name == "objects.hnc.x-k8s.io" { vwc.Webhooks[i].Rules = rules - if err := r.Patch(ctx, vwc, client.MergeFrom(cleanVWC)); err != nil { - return err - } + webhookFound = true break } } - return nil + if !webhookFound { + failurePolicy := apiadmissionregistrationv1.Fail + sideEffects := apiadmissionregistrationv1.SideEffectClassNone + vw := apiadmissionregistrationv1.ValidatingWebhook{ + Name: "objects.hnc.x-k8s.io", + ClientConfig: apiadmissionregistrationv1.WebhookClientConfig{ + Service: &apiadmissionregistrationv1.ServiceReference{ + Namespace: config.GetHNCNamespace(), + Name: "webhook-service", + Path: pointer.String("/validate-objects"), + }, + }, + Rules: rules, + FailurePolicy: &failurePolicy, + SideEffects: &sideEffects, + TimeoutSeconds: pointer.Int32(2), + AdmissionReviewVersions: []string{"v1"}, + // We only apply this object validator on non-excluded namespaces, which have + // the "included-namespace" label set by the HC reconciler, so that when HNC + // (webhook service specifically) is down, operations in the excluded + // namespaces won't be affected. Validators on HNC CRs are not filtered because + // they are supposed to prevent abuse of HNC CRs in excluded namespaces. + // Namespace validator is not filtered to prevent abuse of the included-namespace + // label on excluded namespaces. Unfortunately, this means that when HNC is + // down, we will block updates on all namespaces, even "excluded" ones, but + // anyone who can update namespaces like `kube-system` should likely be able to + // delete the VWHConfiguration to make the updates. + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"hnc.x-k8s.io/included-namespace": "true"}, + }, + } + vwc.Webhooks = append(vwc.Webhooks, vw) + } + + return r.Patch(ctx, vwc, client.MergeFrom(cleanVWC)) } // syncObjectReconcilers creates or syncs ObjectReconcilers. diff --git a/internal/integtest/setup.go b/internal/integtest/setup.go index 071d88d67..ddb639000 100644 --- a/internal/integtest/setup.go +++ b/internal/integtest/setup.go @@ -71,6 +71,10 @@ func HNCBeforeSuite() { By("configuring test environment") testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, + // todo(erikgb): Fix tests that breaks when enabling webhooks + //WebhookInstallOptions: envtest.WebhookInstallOptions{ + // Paths: []string{filepath.Join("..", "..", "config", "webhook")}, + //}, } By("starting test environment") @@ -94,10 +98,14 @@ func HNCBeforeSuite() { // CF: https://github.com/microsoft/azure-databricks-operator/blob/0f722a710fea06b86ecdccd9455336ca712bf775/controllers/suite_test.go By("creating manager") + webhookInstallOptions := &testEnv.WebhookInstallOptions k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ NewClient: config.NewClient(false), MetricsBindAddress: "0", // disable metrics serving since 'go test' runs multiple suites in parallel processes Scheme: scheme.Scheme, + Host: webhookInstallOptions.LocalServingHost, + Port: webhookInstallOptions.LocalServingPort, + CertDir: webhookInstallOptions.LocalServingCertDir, }) Expect(err).ToNot(HaveOccurred()) @@ -111,6 +119,7 @@ func HNCBeforeSuite() { TestForest = forest.NewForest() err = setup.CreateReconcilers(k8sManager, TestForest, opts) Expect(err).ToNot(HaveOccurred()) + setup.CreateWebhooks(k8sManager, TestForest, opts) By("Creating clients") K8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) diff --git a/internal/objects/validator.go b/internal/objects/validator.go index 24e36894f..3820410a0 100644 --- a/internal/objects/validator.go +++ b/internal/objects/validator.go @@ -32,17 +32,6 @@ const ( ServingPath = "/validate-objects" ) -// Note: the validating webhook FAILS CLOSE. This means that if the webhook goes -// down, all further changes are forbidden. In addition, the webhook `rules` -// (groups, resources, versions, verbs) specified in the below kubebuilder marker -// are overwritten by the `rules` configured in config/webhook/webhook_patch.yaml, -// because there's no marker for `scope` and we only want this object webhook -// to work on `namespaced` objects. Please make sure you edit the webhook_patch.yaml -// file if you want to change the webhook `rules` and better make the rules -// here the same as what's in the webhook_patch.yaml. -// -// +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-objects,mutating=false,failurePolicy=fail,groups=,resources=,sideEffects=None,verbs=,versions=,name=objects.hnc.x-k8s.io - type Validator struct { Log logr.Logger Forest *forest.Forest diff --git a/internal/setup/reconcilers.go b/internal/setup/reconcilers.go index c5a458fce..1210884c9 100644 --- a/internal/setup/reconcilers.go +++ b/internal/setup/reconcilers.go @@ -35,7 +35,7 @@ func Create(log logr.Logger, mgr ctrl.Manager, f *forest.Forest, opts Options) { if !opts.NoWebhooks { log.Info("Registering validating webhook (won't work when running locally; use --no-webhooks)") - createWebhooks(mgr, f, opts) + CreateWebhooks(mgr, f, opts) } log.Info("Registering reconcilers") diff --git a/internal/setup/webhooks.go b/internal/setup/webhooks.go index 0a5932170..e4d0a4210 100644 --- a/internal/setup/webhooks.go +++ b/internal/setup/webhooks.go @@ -77,8 +77,8 @@ func ManageCerts(mgr ctrl.Manager, setupFinished chan struct{}, restartOnSecretR }) } -// createWebhooks creates all mutators and validators. -func createWebhooks(mgr ctrl.Manager, f *forest.Forest, opts Options) { +// CreateWebhooks creates all mutators and validators. +func CreateWebhooks(mgr ctrl.Manager, f *forest.Forest, opts Options) { // Create webhook for Hierarchy mgr.GetWebhookServer().Register(hierarchyconfig.ServingPath, &webhook.Admission{Handler: &hierarchyconfig.Validator{ Log: ctrl.Log.WithName("hierarchyconfig").WithName("validate"), From 637630d464b5eb1b5bceaf3875dd0daa89d8ff91 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sun, 28 May 2023 20:23:36 +0200 Subject: [PATCH 05/15] revert attempt to enable webhoks in tests --- internal/integtest/setup.go | 9 --------- internal/setup/reconcilers.go | 2 +- internal/setup/webhooks.go | 4 ++-- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/internal/integtest/setup.go b/internal/integtest/setup.go index ddb639000..071d88d67 100644 --- a/internal/integtest/setup.go +++ b/internal/integtest/setup.go @@ -71,10 +71,6 @@ func HNCBeforeSuite() { By("configuring test environment") testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, - // todo(erikgb): Fix tests that breaks when enabling webhooks - //WebhookInstallOptions: envtest.WebhookInstallOptions{ - // Paths: []string{filepath.Join("..", "..", "config", "webhook")}, - //}, } By("starting test environment") @@ -98,14 +94,10 @@ func HNCBeforeSuite() { // CF: https://github.com/microsoft/azure-databricks-operator/blob/0f722a710fea06b86ecdccd9455336ca712bf775/controllers/suite_test.go By("creating manager") - webhookInstallOptions := &testEnv.WebhookInstallOptions k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ NewClient: config.NewClient(false), MetricsBindAddress: "0", // disable metrics serving since 'go test' runs multiple suites in parallel processes Scheme: scheme.Scheme, - Host: webhookInstallOptions.LocalServingHost, - Port: webhookInstallOptions.LocalServingPort, - CertDir: webhookInstallOptions.LocalServingCertDir, }) Expect(err).ToNot(HaveOccurred()) @@ -119,7 +111,6 @@ func HNCBeforeSuite() { TestForest = forest.NewForest() err = setup.CreateReconcilers(k8sManager, TestForest, opts) Expect(err).ToNot(HaveOccurred()) - setup.CreateWebhooks(k8sManager, TestForest, opts) By("Creating clients") K8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) diff --git a/internal/setup/reconcilers.go b/internal/setup/reconcilers.go index 1210884c9..c5a458fce 100644 --- a/internal/setup/reconcilers.go +++ b/internal/setup/reconcilers.go @@ -35,7 +35,7 @@ func Create(log logr.Logger, mgr ctrl.Manager, f *forest.Forest, opts Options) { if !opts.NoWebhooks { log.Info("Registering validating webhook (won't work when running locally; use --no-webhooks)") - CreateWebhooks(mgr, f, opts) + createWebhooks(mgr, f, opts) } log.Info("Registering reconcilers") diff --git a/internal/setup/webhooks.go b/internal/setup/webhooks.go index e4d0a4210..0a5932170 100644 --- a/internal/setup/webhooks.go +++ b/internal/setup/webhooks.go @@ -77,8 +77,8 @@ func ManageCerts(mgr ctrl.Manager, setupFinished chan struct{}, restartOnSecretR }) } -// CreateWebhooks creates all mutators and validators. -func CreateWebhooks(mgr ctrl.Manager, f *forest.Forest, opts Options) { +// createWebhooks creates all mutators and validators. +func createWebhooks(mgr ctrl.Manager, f *forest.Forest, opts Options) { // Create webhook for Hierarchy mgr.GetWebhookServer().Register(hierarchyconfig.ServingPath, &webhook.Admission{Handler: &hierarchyconfig.Validator{ Log: ctrl.Log.WithName("hierarchyconfig").WithName("validate"), From f04e447fdfb07432515eb9ae988a4e484849fe0e Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sun, 11 Jun 2023 15:59:25 +0200 Subject: [PATCH 06/15] initial webhook --- config/webhook/manifests.yaml | 22 ++++++++++++++++++++++ internal/objects/validator.go | 6 ++++++ 2 files changed, 28 insertions(+) diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index b2cb0193c..8d66c4371 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -71,6 +71,28 @@ webhooks: resources: - hierarchyconfigurations sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-objects + failurePolicy: Fail + name: objects.hnc.x-k8s.io + rules: + - apiGroups: + - rbac.authorization.k8s.io + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + - DELETE + resources: + - roles + - rolebindings + sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/internal/objects/validator.go b/internal/objects/validator.go index 3820410a0..b1601f92d 100644 --- a/internal/objects/validator.go +++ b/internal/objects/validator.go @@ -32,6 +32,12 @@ const ( ServingPath = "/validate-objects" ) +// Note: the validating webhook FAILS CLOSE. This means that if the webhook goes +// down, all further changes are forbidden. The initial webhook configuration contains +// just enforced types, and will be dynamically updated when reconciling the HNC configuration. +// +// +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-objects,mutating=false,failurePolicy=fail,groups="rbac.authorization.k8s.io",resources=roles;rolebindings,sideEffects=None,verbs=create;update;delete,versions=v1,name=objects.hnc.x-k8s.io + type Validator struct { Log logr.Logger Forest *forest.Forest From 07a8a95fb98c8d42e63f20e5d5e4bc3d67754f25 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sun, 11 Jun 2023 16:40:23 +0200 Subject: [PATCH 07/15] Better webhook structure --- config/webhook/manifests.yaml | 2 +- internal/hncconfig/reconciler.go | 59 +++++++------------------------- internal/objects/validator.go | 2 +- internal/setup/webhooks.go | 4 +-- internal/webhooks/webhooks.go | 6 ++-- 5 files changed, 20 insertions(+), 53 deletions(-) diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 8d66c4371..fd57f3f0d 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -84,7 +84,7 @@ webhooks: - apiGroups: - rbac.authorization.k8s.io apiVersions: - - v1 + - '*' operations: - CREATE - UPDATE diff --git a/internal/hncconfig/reconciler.go b/internal/hncconfig/reconciler.go index 9493bc777..70a5225a7 100644 --- a/internal/hncconfig/reconciler.go +++ b/internal/hncconfig/reconciler.go @@ -15,7 +15,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" @@ -25,7 +24,6 @@ import ( api "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2" "sigs.k8s.io/hierarchical-namespaces/internal/apimeta" - "sigs.k8s.io/hierarchical-namespaces/internal/config" "sigs.k8s.io/hierarchical-namespaces/internal/crd" "sigs.k8s.io/hierarchical-namespaces/internal/forest" "sigs.k8s.io/hierarchical-namespaces/internal/objects" @@ -241,21 +239,24 @@ func (r *Reconciler) writeSingleton(ctx context.Context, inst *api.HNCConfigurat } func (r *Reconciler) syncObjectWebhookConfigs(ctx context.Context) error { - namespacedScope := apiadmissionregistrationv1.NamespacedScope + // Group GR by group + groups := make(map[string][]string) + for gr := range r.activeGVKMode { + groups[gr.Group] = append(groups[gr.Group], gr.Resource) + } var rules []apiadmissionregistrationv1.RuleWithOperations - for gr := range r.activeGVKMode { + for g, res := range groups { rule := apiadmissionregistrationv1.RuleWithOperations{} - rule.APIGroups = []string{gr.Group} - rule.Resources = []string{gr.Resource} + rule.APIGroups = []string{g} + rule.Resources = res rule.APIVersions = []string{"*"} - rule.Scope = &namespacedScope rule.Operations = []apiadmissionregistrationv1.OperationType{apiadmissionregistrationv1.Create, apiadmissionregistrationv1.Update, apiadmissionregistrationv1.Delete} rules = append(rules, rule) } vwc := &apiadmissionregistrationv1.ValidatingWebhookConfiguration{} - if err := r.Get(ctx, client.ObjectKey{Name: webhooks.ValidatingWebhookName}, vwc); err != nil { + if err := r.Get(ctx, client.ObjectKey{Name: webhooks.ValidatingWebhookConfigurationName}, vwc); err != nil { if errors.IsNotFound(err) { // todo(erikgb): See if the tests can/should be bootstrapped with this webhook // Webhook not found; nothing to reconcile @@ -265,49 +266,13 @@ func (r *Reconciler) syncObjectWebhookConfigs(ctx context.Context) error { } cleanVWC := vwc.DeepCopy() - webhookFound := false for i, wh := range vwc.Webhooks { - if wh.Name == "objects.hnc.x-k8s.io" { + if wh.Name == webhooks.ObjectsWebhookName { vwc.Webhooks[i].Rules = rules - webhookFound = true - break + return r.Patch(ctx, vwc, client.MergeFrom(cleanVWC)) } } - if !webhookFound { - failurePolicy := apiadmissionregistrationv1.Fail - sideEffects := apiadmissionregistrationv1.SideEffectClassNone - vw := apiadmissionregistrationv1.ValidatingWebhook{ - Name: "objects.hnc.x-k8s.io", - ClientConfig: apiadmissionregistrationv1.WebhookClientConfig{ - Service: &apiadmissionregistrationv1.ServiceReference{ - Namespace: config.GetHNCNamespace(), - Name: "webhook-service", - Path: pointer.String("/validate-objects"), - }, - }, - Rules: rules, - FailurePolicy: &failurePolicy, - SideEffects: &sideEffects, - TimeoutSeconds: pointer.Int32(2), - AdmissionReviewVersions: []string{"v1"}, - // We only apply this object validator on non-excluded namespaces, which have - // the "included-namespace" label set by the HC reconciler, so that when HNC - // (webhook service specifically) is down, operations in the excluded - // namespaces won't be affected. Validators on HNC CRs are not filtered because - // they are supposed to prevent abuse of HNC CRs in excluded namespaces. - // Namespace validator is not filtered to prevent abuse of the included-namespace - // label on excluded namespaces. Unfortunately, this means that when HNC is - // down, we will block updates on all namespaces, even "excluded" ones, but - // anyone who can update namespaces like `kube-system` should likely be able to - // delete the VWHConfiguration to make the updates. - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"hnc.x-k8s.io/included-namespace": "true"}, - }, - } - vwc.Webhooks = append(vwc.Webhooks, vw) - } - - return r.Patch(ctx, vwc, client.MergeFrom(cleanVWC)) + return fmt.Errorf("webhook %q not found in ValidatingWebhookConfiguration %q", webhooks.ObjectsWebhookName, webhooks.ValidatingWebhookConfigurationName) } // syncObjectReconcilers creates or syncs ObjectReconcilers. diff --git a/internal/objects/validator.go b/internal/objects/validator.go index b1601f92d..84b4d2d15 100644 --- a/internal/objects/validator.go +++ b/internal/objects/validator.go @@ -36,7 +36,7 @@ const ( // down, all further changes are forbidden. The initial webhook configuration contains // just enforced types, and will be dynamically updated when reconciling the HNC configuration. // -// +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-objects,mutating=false,failurePolicy=fail,groups="rbac.authorization.k8s.io",resources=roles;rolebindings,sideEffects=None,verbs=create;update;delete,versions=v1,name=objects.hnc.x-k8s.io +// +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-objects,mutating=false,failurePolicy=fail,groups="rbac.authorization.k8s.io",resources=roles;rolebindings,sideEffects=None,verbs=create;update;delete,versions="*",name=objects.hnc.x-k8s.io type Validator struct { Log logr.Logger diff --git a/internal/setup/webhooks.go b/internal/setup/webhooks.go index 0a5932170..6c3849657 100644 --- a/internal/setup/webhooks.go +++ b/internal/setup/webhooks.go @@ -48,10 +48,10 @@ func ManageCerts(mgr ctrl.Manager, setupFinished chan struct{}, restartOnSecretR IsReady: setupFinished, Webhooks: []cert.WebhookInfo{{ Type: cert.Validating, - Name: webhooks.ValidatingWebhookName, + Name: webhooks.ValidatingWebhookConfigurationName, }, { Type: cert.Mutating, - Name: webhooks.MutatingWebhookName, + Name: webhooks.MutatingWebhookConfigurationName, }}, RestartOnSecretRefresh: restartOnSecretRefresh, }) diff --git a/internal/webhooks/webhooks.go b/internal/webhooks/webhooks.go index a7abf9ca1..32d703d07 100644 --- a/internal/webhooks/webhooks.go +++ b/internal/webhooks/webhooks.go @@ -15,8 +15,10 @@ import ( ) const ( - ValidatingWebhookName = "hnc-validating-webhook-configuration" - MutatingWebhookName = "hnc-mutating-webhook-configuration" + ValidatingWebhookConfigurationName = "hnc-validating-webhook-configuration" + MutatingWebhookConfigurationName = "hnc-mutating-webhook-configuration" + + ObjectsWebhookName = "objects.hnc.x-k8s.io" ) // IsHNCServiceAccount is inspired by isGKServiceAccount from open-policy-agent/gatekeeper. From a42edacaff723a27d0b297f8c71786fa49e72ae9 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sun, 11 Jun 2023 17:54:24 +0200 Subject: [PATCH 08/15] Create webhook without rules by patch --- config/webhook/kustomization.yaml | 3 +++ config/webhook/manifests.yaml | 22 ---------------------- config/webhook/webhook_patch.yaml | 31 +++++++++++++++++++++++++++++++ internal/objects/validator.go | 6 ------ 4 files changed, 34 insertions(+), 28 deletions(-) create mode 100644 config/webhook/webhook_patch.yaml diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml index 9cf26134e..eb3ccebfe 100644 --- a/config/webhook/kustomization.yaml +++ b/config/webhook/kustomization.yaml @@ -4,3 +4,6 @@ resources: configurations: - kustomizeconfig.yaml + +patchesStrategicMerge: +- webhook_patch.yaml diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index fd57f3f0d..b2cb0193c 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -71,28 +71,6 @@ webhooks: resources: - hierarchyconfigurations sideEffects: None -- admissionReviewVersions: - - v1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /validate-objects - failurePolicy: Fail - name: objects.hnc.x-k8s.io - rules: - - apiGroups: - - rbac.authorization.k8s.io - apiVersions: - - '*' - operations: - - CREATE - - UPDATE - - DELETE - resources: - - roles - - rolebindings - sideEffects: None - admissionReviewVersions: - v1 clientConfig: diff --git a/config/webhook/webhook_patch.yaml b/config/webhook/webhook_patch.yaml new file mode 100644 index 000000000..4c45eec89 --- /dev/null +++ b/config/webhook/webhook_patch.yaml @@ -0,0 +1,31 @@ +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: validating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-objects + failurePolicy: Fail + name: objects.hnc.x-k8s.io + # Intentionally no rules, as these are maintained by the HNCConfiguration reconciler + sideEffects: None + timeoutSeconds: 2 + # We only apply this object validator on non-excluded namespaces, which have + # the "included-namespace" label set by the HC reconciler, so that when HNC + # (webhook service specifically) is down, operations in the excluded + # namespaces won't be affected. Validators on HNC CRs are not filtered because + # they are supposed to prevent abuse of HNC CRs in excluded namespaces. + # Namespace validator is not filtered to prevent abuse of the included-namespace + # label on excluded namespaces. Unfortunately, this means that when HNC is + # down, we will block updates on all namespaces, even "excluded" ones, but + # anyone who can update namespaces like `kube-system` should likely be able to + # delete the VWHConfiguration to make the updates. + namespaceSelector: + matchLabels: + hnc.x-k8s.io/included-namespace: "true" diff --git a/internal/objects/validator.go b/internal/objects/validator.go index 84b4d2d15..3820410a0 100644 --- a/internal/objects/validator.go +++ b/internal/objects/validator.go @@ -32,12 +32,6 @@ const ( ServingPath = "/validate-objects" ) -// Note: the validating webhook FAILS CLOSE. This means that if the webhook goes -// down, all further changes are forbidden. The initial webhook configuration contains -// just enforced types, and will be dynamically updated when reconciling the HNC configuration. -// -// +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-objects,mutating=false,failurePolicy=fail,groups="rbac.authorization.k8s.io",resources=roles;rolebindings,sideEffects=None,verbs=create;update;delete,versions="*",name=objects.hnc.x-k8s.io - type Validator struct { Log logr.Logger Forest *forest.Forest From db236ea20001caa2eb0aaddfaf67ac951c282cd7 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sun, 11 Jun 2023 17:55:52 +0200 Subject: [PATCH 09/15] Move comment --- config/webhook/webhook_patch.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/webhook/webhook_patch.yaml b/config/webhook/webhook_patch.yaml index 4c45eec89..c54bf3ab1 100644 --- a/config/webhook/webhook_patch.yaml +++ b/config/webhook/webhook_patch.yaml @@ -13,7 +13,6 @@ webhooks: path: /validate-objects failurePolicy: Fail name: objects.hnc.x-k8s.io - # Intentionally no rules, as these are maintained by the HNCConfiguration reconciler sideEffects: None timeoutSeconds: 2 # We only apply this object validator on non-excluded namespaces, which have @@ -29,3 +28,4 @@ webhooks: namespaceSelector: matchLabels: hnc.x-k8s.io/included-namespace: "true" + # Intentionally no rules, as these are maintained by the HNCConfiguration reconciler From ebe7df4bdc0d91a354f79f71337ffe1885214af7 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Fri, 16 Jun 2023 22:01:49 +0200 Subject: [PATCH 10/15] Improve code --- internal/hncconfig/reconciler.go | 37 +++++++++++++++++--------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/internal/hncconfig/reconciler.go b/internal/hncconfig/reconciler.go index 70a5225a7..ac50bdec7 100644 --- a/internal/hncconfig/reconciler.go +++ b/internal/hncconfig/reconciler.go @@ -239,22 +239,6 @@ func (r *Reconciler) writeSingleton(ctx context.Context, inst *api.HNCConfigurat } func (r *Reconciler) syncObjectWebhookConfigs(ctx context.Context) error { - // Group GR by group - groups := make(map[string][]string) - for gr := range r.activeGVKMode { - groups[gr.Group] = append(groups[gr.Group], gr.Resource) - } - - var rules []apiadmissionregistrationv1.RuleWithOperations - for g, res := range groups { - rule := apiadmissionregistrationv1.RuleWithOperations{} - rule.APIGroups = []string{g} - rule.Resources = res - rule.APIVersions = []string{"*"} - rule.Operations = []apiadmissionregistrationv1.OperationType{apiadmissionregistrationv1.Create, apiadmissionregistrationv1.Update, apiadmissionregistrationv1.Delete} - rules = append(rules, rule) - } - vwc := &apiadmissionregistrationv1.ValidatingWebhookConfiguration{} if err := r.Get(ctx, client.ObjectKey{Name: webhooks.ValidatingWebhookConfigurationName}, vwc); err != nil { if errors.IsNotFound(err) { @@ -268,13 +252,32 @@ func (r *Reconciler) syncObjectWebhookConfigs(ctx context.Context) error { for i, wh := range vwc.Webhooks { if wh.Name == webhooks.ObjectsWebhookName { - vwc.Webhooks[i].Rules = rules + vwc.Webhooks[i].Rules = objectWebhookRules(r.activeGVKMode) return r.Patch(ctx, vwc, client.MergeFrom(cleanVWC)) } } return fmt.Errorf("webhook %q not found in ValidatingWebhookConfiguration %q", webhooks.ObjectsWebhookName, webhooks.ValidatingWebhookConfigurationName) } +func objectWebhookRules(mode gr2gvkMode) []apiadmissionregistrationv1.RuleWithOperations { + // Group GR by group to make nicer rules + g2r := make(map[string][]string) + for gr := range mode { + g2r[gr.Group] = append(g2r[gr.Group], gr.Resource) + } + + var rules []apiadmissionregistrationv1.RuleWithOperations + for g, r := range g2r { + rule := apiadmissionregistrationv1.RuleWithOperations{} + rule.APIGroups = []string{g} + rule.Resources = r + rule.APIVersions = []string{"*"} + rule.Operations = []apiadmissionregistrationv1.OperationType{apiadmissionregistrationv1.Create, apiadmissionregistrationv1.Update, apiadmissionregistrationv1.Delete} + rules = append(rules, rule) + } + return rules +} + // syncObjectReconcilers creates or syncs ObjectReconcilers. // // For newly added types in the HNC configuration, the method will create corresponding ObjectReconcilers and From bcfffac60d4597a0638eb64ea3252dc70abf7bf6 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Fri, 16 Jun 2023 23:17:31 +0200 Subject: [PATCH 11/15] Include objects webhook in test control plane --- internal/hncconfig/reconciler.go | 5 ---- internal/integtest/setup.go | 40 ++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/internal/hncconfig/reconciler.go b/internal/hncconfig/reconciler.go index ac50bdec7..4b06a9ee4 100644 --- a/internal/hncconfig/reconciler.go +++ b/internal/hncconfig/reconciler.go @@ -241,11 +241,6 @@ func (r *Reconciler) writeSingleton(ctx context.Context, inst *api.HNCConfigurat func (r *Reconciler) syncObjectWebhookConfigs(ctx context.Context) error { vwc := &apiadmissionregistrationv1.ValidatingWebhookConfiguration{} if err := r.Get(ctx, client.ObjectKey{Name: webhooks.ValidatingWebhookConfigurationName}, vwc); err != nil { - if errors.IsNotFound(err) { - // todo(erikgb): See if the tests can/should be bootstrapped with this webhook - // Webhook not found; nothing to reconcile - return nil - } return err } cleanVWC := vwc.DeepCopy() diff --git a/internal/integtest/setup.go b/internal/integtest/setup.go index 071d88d67..302952ee0 100644 --- a/internal/integtest/setup.go +++ b/internal/integtest/setup.go @@ -23,14 +23,21 @@ import ( . "github.com/onsi/ginkgo/v2" //lint:ignore ST1001 Ignoring this for now . "github.com/onsi/gomega" //lint:ignore ST1001 Ignoring this for now + apiadmissionregistrationv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "sigs.k8s.io/hierarchical-namespaces/internal/objects" + "sigs.k8s.io/hierarchical-namespaces/internal/webhooks" // +kubebuilder:scaffold:imports @@ -69,8 +76,28 @@ func HNCBeforeSuite() { SetDefaultEventuallyTimeout(time.Second * 4) By("configuring test environment") + sideEffectClassNone := apiadmissionregistrationv1.SideEffectClassNone testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, + WebhookInstallOptions: envtest.WebhookInstallOptions{ + ValidatingWebhooks: []*apiadmissionregistrationv1.ValidatingWebhookConfiguration{{ + ObjectMeta: metav1.ObjectMeta{ + Name: webhooks.ValidatingWebhookConfigurationName, + }, + Webhooks: []apiadmissionregistrationv1.ValidatingWebhook{{ + Name: webhooks.ObjectsWebhookName, + AdmissionReviewVersions: []string{"v1"}, + SideEffects: &sideEffectClassNone, + ClientConfig: apiadmissionregistrationv1.WebhookClientConfig{ + Service: &apiadmissionregistrationv1.ServiceReference{ + Namespace: "system", + Name: "webhook-service", + Path: pointer.String(objects.ServingPath), + }, + }, + }}, + }}, + }, } By("starting test environment") @@ -94,13 +121,20 @@ func HNCBeforeSuite() { // CF: https://github.com/microsoft/azure-databricks-operator/blob/0f722a710fea06b86ecdccd9455336ca712bf775/controllers/suite_test.go By("creating manager") + webhookInstallOptions := &testEnv.WebhookInstallOptions k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ NewClient: config.NewClient(false), MetricsBindAddress: "0", // disable metrics serving since 'go test' runs multiple suites in parallel processes Scheme: scheme.Scheme, + Host: webhookInstallOptions.LocalServingHost, + Port: webhookInstallOptions.LocalServingPort, + CertDir: webhookInstallOptions.LocalServingCertDir, }) Expect(err).ToNot(HaveOccurred()) + // Register a dummy webhook since the test control plane is to test reconcilers + k8sManager.GetWebhookServer().Register(objects.ServingPath, &webhook.Admission{Handler: &allowAllHandler{}}) + By("creating reconcilers") opts := setup.Options{ MaxReconciles: 100, @@ -125,6 +159,12 @@ func HNCBeforeSuite() { }() } +type allowAllHandler struct{} + +func (a allowAllHandler) Handle(_ context.Context, _ admission.Request) admission.Response { + return webhooks.Allow("All requests are allowed by allowAllHandler") +} + func HNCAfterSuite() { if k8sManagerCancelFn != nil { k8sManagerCancelFn() From 6e89f15f9e8b840f801c33e55e057bc35a09570b Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Fri, 16 Jun 2023 23:28:59 +0200 Subject: [PATCH 12/15] Update webhook patch comments --- config/webhook/webhook_patch.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/webhook/webhook_patch.yaml b/config/webhook/webhook_patch.yaml index c54bf3ab1..ddb67a011 100644 --- a/config/webhook/webhook_patch.yaml +++ b/config/webhook/webhook_patch.yaml @@ -28,4 +28,5 @@ webhooks: namespaceSelector: matchLabels: hnc.x-k8s.io/included-namespace: "true" - # Intentionally no rules, as these are maintained by the HNCConfiguration reconciler + # Intentionally no rules, as these are maintained by the HNCConfiguration reconciler. + # At present controller-gen is unable to generate a webhook without rules from kubebuilder markers. From 3cdf2a760c9e3673acd1943c83b81f4a2ebbdb8b Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sun, 10 Mar 2024 11:53:28 +0100 Subject: [PATCH 13/15] move comment to top --- config/webhook/webhook_patch.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/webhook/webhook_patch.yaml b/config/webhook/webhook_patch.yaml index ddb67a011..f3c8c6b88 100644 --- a/config/webhook/webhook_patch.yaml +++ b/config/webhook/webhook_patch.yaml @@ -1,4 +1,6 @@ --- +# Intentionally no rules, as these are maintained by the HNCConfiguration reconciler. +# At present controller-gen is unable to generate a webhook without rules from kubebuilder markers. apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: @@ -28,5 +30,3 @@ webhooks: namespaceSelector: matchLabels: hnc.x-k8s.io/included-namespace: "true" - # Intentionally no rules, as these are maintained by the HNCConfiguration reconciler. - # At present controller-gen is unable to generate a webhook without rules from kubebuilder markers. From 17d0f3edcce64999217b3753e3ddd4c05b1759d3 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sun, 10 Mar 2024 11:58:04 +0100 Subject: [PATCH 14/15] revert changes to integration test setup --- internal/integtest/setup.go | 40 ------------------------------------- 1 file changed, 40 deletions(-) diff --git a/internal/integtest/setup.go b/internal/integtest/setup.go index 302952ee0..071d88d67 100644 --- a/internal/integtest/setup.go +++ b/internal/integtest/setup.go @@ -23,21 +23,14 @@ import ( . "github.com/onsi/ginkgo/v2" //lint:ignore ST1001 Ignoring this for now . "github.com/onsi/gomega" //lint:ignore ST1001 Ignoring this for now - apiadmissionregistrationv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" - "sigs.k8s.io/controller-runtime/pkg/webhook" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "sigs.k8s.io/hierarchical-namespaces/internal/objects" - "sigs.k8s.io/hierarchical-namespaces/internal/webhooks" // +kubebuilder:scaffold:imports @@ -76,28 +69,8 @@ func HNCBeforeSuite() { SetDefaultEventuallyTimeout(time.Second * 4) By("configuring test environment") - sideEffectClassNone := apiadmissionregistrationv1.SideEffectClassNone testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, - WebhookInstallOptions: envtest.WebhookInstallOptions{ - ValidatingWebhooks: []*apiadmissionregistrationv1.ValidatingWebhookConfiguration{{ - ObjectMeta: metav1.ObjectMeta{ - Name: webhooks.ValidatingWebhookConfigurationName, - }, - Webhooks: []apiadmissionregistrationv1.ValidatingWebhook{{ - Name: webhooks.ObjectsWebhookName, - AdmissionReviewVersions: []string{"v1"}, - SideEffects: &sideEffectClassNone, - ClientConfig: apiadmissionregistrationv1.WebhookClientConfig{ - Service: &apiadmissionregistrationv1.ServiceReference{ - Namespace: "system", - Name: "webhook-service", - Path: pointer.String(objects.ServingPath), - }, - }, - }}, - }}, - }, } By("starting test environment") @@ -121,20 +94,13 @@ func HNCBeforeSuite() { // CF: https://github.com/microsoft/azure-databricks-operator/blob/0f722a710fea06b86ecdccd9455336ca712bf775/controllers/suite_test.go By("creating manager") - webhookInstallOptions := &testEnv.WebhookInstallOptions k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ NewClient: config.NewClient(false), MetricsBindAddress: "0", // disable metrics serving since 'go test' runs multiple suites in parallel processes Scheme: scheme.Scheme, - Host: webhookInstallOptions.LocalServingHost, - Port: webhookInstallOptions.LocalServingPort, - CertDir: webhookInstallOptions.LocalServingCertDir, }) Expect(err).ToNot(HaveOccurred()) - // Register a dummy webhook since the test control plane is to test reconcilers - k8sManager.GetWebhookServer().Register(objects.ServingPath, &webhook.Admission{Handler: &allowAllHandler{}}) - By("creating reconcilers") opts := setup.Options{ MaxReconciles: 100, @@ -159,12 +125,6 @@ func HNCBeforeSuite() { }() } -type allowAllHandler struct{} - -func (a allowAllHandler) Handle(_ context.Context, _ admission.Request) admission.Response { - return webhooks.Allow("All requests are allowed by allowAllHandler") -} - func HNCAfterSuite() { if k8sManagerCancelFn != nil { k8sManagerCancelFn() From 36139e304f84698a802bdf1ca4fb1d7f92c3dc32 Mon Sep 17 00:00:00 2001 From: Erik Godding Boye Date: Sun, 10 Mar 2024 13:30:13 +0100 Subject: [PATCH 15/15] Fix rebase errors --- internal/integtest/setup.go | 40 +++++++++++++++++++++++++++++++++++++ internal/setup/webhooks.go | 9 +++++---- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/internal/integtest/setup.go b/internal/integtest/setup.go index 071d88d67..302952ee0 100644 --- a/internal/integtest/setup.go +++ b/internal/integtest/setup.go @@ -23,14 +23,21 @@ import ( . "github.com/onsi/ginkgo/v2" //lint:ignore ST1001 Ignoring this for now . "github.com/onsi/gomega" //lint:ignore ST1001 Ignoring this for now + apiadmissionregistrationv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + "sigs.k8s.io/hierarchical-namespaces/internal/objects" + "sigs.k8s.io/hierarchical-namespaces/internal/webhooks" // +kubebuilder:scaffold:imports @@ -69,8 +76,28 @@ func HNCBeforeSuite() { SetDefaultEventuallyTimeout(time.Second * 4) By("configuring test environment") + sideEffectClassNone := apiadmissionregistrationv1.SideEffectClassNone testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, + WebhookInstallOptions: envtest.WebhookInstallOptions{ + ValidatingWebhooks: []*apiadmissionregistrationv1.ValidatingWebhookConfiguration{{ + ObjectMeta: metav1.ObjectMeta{ + Name: webhooks.ValidatingWebhookConfigurationName, + }, + Webhooks: []apiadmissionregistrationv1.ValidatingWebhook{{ + Name: webhooks.ObjectsWebhookName, + AdmissionReviewVersions: []string{"v1"}, + SideEffects: &sideEffectClassNone, + ClientConfig: apiadmissionregistrationv1.WebhookClientConfig{ + Service: &apiadmissionregistrationv1.ServiceReference{ + Namespace: "system", + Name: "webhook-service", + Path: pointer.String(objects.ServingPath), + }, + }, + }}, + }}, + }, } By("starting test environment") @@ -94,13 +121,20 @@ func HNCBeforeSuite() { // CF: https://github.com/microsoft/azure-databricks-operator/blob/0f722a710fea06b86ecdccd9455336ca712bf775/controllers/suite_test.go By("creating manager") + webhookInstallOptions := &testEnv.WebhookInstallOptions k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ NewClient: config.NewClient(false), MetricsBindAddress: "0", // disable metrics serving since 'go test' runs multiple suites in parallel processes Scheme: scheme.Scheme, + Host: webhookInstallOptions.LocalServingHost, + Port: webhookInstallOptions.LocalServingPort, + CertDir: webhookInstallOptions.LocalServingCertDir, }) Expect(err).ToNot(HaveOccurred()) + // Register a dummy webhook since the test control plane is to test reconcilers + k8sManager.GetWebhookServer().Register(objects.ServingPath, &webhook.Admission{Handler: &allowAllHandler{}}) + By("creating reconcilers") opts := setup.Options{ MaxReconciles: 100, @@ -125,6 +159,12 @@ func HNCBeforeSuite() { }() } +type allowAllHandler struct{} + +func (a allowAllHandler) Handle(_ context.Context, _ admission.Request) admission.Response { + return webhooks.Allow("All requests are allowed by allowAllHandler") +} + func HNCAfterSuite() { if k8sManagerCancelFn != nil { k8sManagerCancelFn() diff --git a/internal/setup/webhooks.go b/internal/setup/webhooks.go index 6c3849657..ed2a13de6 100644 --- a/internal/setup/webhooks.go +++ b/internal/setup/webhooks.go @@ -21,10 +21,11 @@ import ( const ( serviceName = "hnc-webhook-service" - caName = "hnc-ca" - caOrganization = "hnc" - secretName = "hnc-webhook-server-cert" - certDir = "/tmp/k8s-webhook-server/serving-certs"apiExtCertDir = "/certs" + caName = "hnc-ca" + caOrganization = "hnc" + secretName = "hnc-webhook-server-cert" + certDir = "/tmp/k8s-webhook-server/serving-certs" + apiExtCertDir = "/certs" apiExtServiceName = "hnc-resourcelist" apiExtSecretName = "hnc-resourcelist" apiExtName = "v1alpha2.resources.hnc.x-k8s.io"