Skip to content

Commit

Permalink
Add webhook for limit 64 char name+np
Browse files Browse the repository at this point in the history
- The 63 character limit validation for policy name is not working while creating a Policy via CLI.
- Policy creation - Policy name + namespace name can’t go longer than 63
From CLI, it possible to create a policy with a name like “policy-check-setupjob-setupjob-operator-installer” with no issue; but, while wanting to edit it, we can’t.
- Getting this error: “The combined length of namespace and policy nmabe (namespaceName.policyName) should not exceed 63 characters (see attached screeshot)

Ref: https://issues.redhat.com/browse/ACM-3558

Signed-off-by: Yi Rae Kim <yikim@redhat.com>
  • Loading branch information
yiraeChristineKim authored and openshift-merge-robot committed Jul 19, 2023
1 parent 4f275f8 commit c93f938
Show file tree
Hide file tree
Showing 13 changed files with 385 additions and 6 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/kind.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,13 @@ jobs:
- name: Verify Deployment Configuration
run: |
make webhook
make build-images
make kind-deploy-controller-dev
- name: E2E Tests for Webhook
run: |
make e2e-test-webhook
- name: Debug
if: ${{ failure() }}
Expand All @@ -93,3 +98,4 @@ jobs:
if: ${{ always() }}
run: |
make kind-delete-cluster
17 changes: 15 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,21 @@ kustomize: ## Download kustomize locally if necessary.
GINKGO = $(LOCAL_BIN)/ginkgo

.PHONY: kind-bootstrap-cluster
kind-bootstrap-cluster: kind-create-cluster install-crds kind-deploy-controller install-resources
kind-bootstrap-cluster: kind-create-cluster install-crds webhook kind-deploy-controller install-resources

.PHONY: kind-bootstrap-cluster-dev
kind-bootstrap-cluster-dev: kind-create-cluster install-crds install-resources

webhook:
-kubectl create ns $(KIND_NAMESPACE)
@echo installing cert-manager
kubectl apply -f https://github.com/cert-manager/cert-manager/releases/download/v1.12.0/cert-manager.yaml
@echo "wait until pods are up"
kubectl wait deployment -n cert-manager cert-manager --for condition=Available=True --timeout=90s
kubectl wait --for=condition=Ready pod -l app.kubernetes.io/instance=cert-manager -n cert-manager --timeout=30s
sed 's/namespace: open-cluster-management/namespace: $(KIND_NAMESPACE)/g' deploy/webhook.yaml | kubectl apply -f-


.PHONY: kind-deploy-controller
kind-deploy-controller: manifests
@echo installing $(IMG)
Expand Down Expand Up @@ -277,7 +287,7 @@ e2e-dependencies:

.PHONY: e2e-test
e2e-test: e2e-dependencies
$(GINKGO) -v --fail-fast $(E2E_TEST_ARGS) test/e2e
$(GINKGO) -v --fail-fast $(E2E_TEST_ARGS) --label-filter="!webhook" test/e2e

.PHONY: e2e-test-coverage
e2e-test-coverage: E2E_TEST_ARGS = --json-report=report_e2e.json --output-dir=.
Expand All @@ -295,6 +305,9 @@ e2e-run-instrumented: e2e-build-instrumented
e2e-stop-instrumented:
ps -ef | grep '$(IMG)' | grep -v grep | awk '{print $$2}' | xargs kill

e2e-test-webhook:
$(GINKGO) -v --fail-fast --label-filter="webhook" test/e2e

.PHONY: e2e-debug
e2e-debug:
@echo local controller log:
Expand Down
3 changes: 3 additions & 0 deletions PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ resources:
group: policy
kind: Policy
path: open-cluster-management.io/governance-policy-propagator/api/v1
webhooks:
validation: true
webhookVersion: v1
version: v1
- api:
crdVersion: v1
Expand Down
12 changes: 9 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,15 @@ make e2e-dependencies
make e2e-test
```

### How to run webhook locally

```bash
--enable-webhooks=true
```

> **Limit**: If you want to run the webhook locally, you need to generate certificates and place them in `/tmp/k8s-webhook-server/serving-certs/tls.{crt,key}`. If you’re not running a local API server, you’ll also need to figure out how to proxy traffic from the remote cluster to your local webhook server. For this reason, Kubebuilder generally recommends disabling webhooks when doing your local code-run-test cycle. To disable it, please supply the `--enable-webhooks=false` argument when running the controller.
> For more information, visit https://book.kubebuilder.io/cronjob-tutorial/running.html
### Clean up

```
Expand Down Expand Up @@ -92,6 +101,3 @@ The following environment variables can be set to configure the controller:
- The `governance-policy-propagator` is part of the `open-cluster-management` community. For more information, visit:
[open-cluster-management.io](https://open-cluster-management.io).

<!---
Date: 2022-03-2
-->
66 changes: 66 additions & 0 deletions api/v1/policy_webhook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright (c) 2021 Red Hat, Inc.
// Copyright Contributors to the Open Cluster Management project

package v1

import (
"errors"
"unicode/utf8"

"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

var (
// log is for logging in this package.
policylog = logf.Log.WithName("policy-validating-webhook")
errName = errors.New("the combined length of the policy namespace and name " +
"<namespace>.<name> cannot exceed 63 characters")
)

func (r *Policy) SetupWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).
For(r).
Complete()
}

//+kubebuilder:webhook:path=/validate-policy-open-cluster-management-io-v1-policy,mutating=false,failurePolicy=Ignore,sideEffects=None,groups=policy.open-cluster-management.io,resources=policies,verbs=create,versions=v1,name=policy.open-cluster-management.io.webhook,admissionReviewVersions=v1

var _ webhook.Validator = &Policy{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
func (r *Policy) ValidateCreate() (admission.Warnings, error) {
policylog.Info("Validate policy creation request", "name", r.Name)

return r.validateName()
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (r *Policy) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) {
return nil, nil
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
func (r *Policy) ValidateDelete() (admission.Warnings, error) {
return nil, nil
}

// validate the policy name and namespace length
func (r *Policy) validateName() (admission.Warnings, error) {
policylog.Info("Validating the policy name through a validating webhook")

// replicated policies don't need pass this validation
if _, ok := r.GetLabels()["policy.open-cluster-management.io/root-policy"]; ok {
return nil, nil
}

// 1 character for "."
if (utf8.RuneCountInString(r.Name) + utf8.RuneCountInString(r.Namespace)) > 62 {
return nil, errName
}

return nil, nil
}
18 changes: 18 additions & 0 deletions deploy/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,22 @@
apiVersion: apps/v1
kind: Deployment
metadata:
labels:
webhook-origin: governance-policy-propagator
name: governance-policy-propagator
spec:
replicas: 1
selector:
matchLabels:
name: governance-policy-propagator
webhook-origin: governance-policy-propagator
template:
metadata:
annotations:
kubectl.kubernetes.io/default-container: governance-policy-propagator
labels:
name: governance-policy-propagator
webhook-origin: governance-policy-propagator
spec:
serviceAccountName: governance-policy-propagator
containers:
Expand All @@ -27,7 +33,14 @@ spec:
- containerPort: 8383
protocol: TCP
name: http
- containerPort: 9443
protocol: TCP
name: webhook-http
imagePullPolicy: Always
volumeMounts:
- mountPath: /tmp/k8s-webhook-server/serving-certs
name: cert
readOnly: true
env:
- name: WATCH_NAMESPACE
value: ""
Expand All @@ -37,6 +50,11 @@ spec:
fieldPath: metadata.name
- name: OPERATOR_NAME
value: "governance-policy-propagator"
volumes:
- name: cert
secret:
defaultMode: 420
secretName: propagator-webhook-server-cert
---
kind: ClusterRoleBinding
apiVersion: rbac.authorization.k8s.io/v1
Expand Down
18 changes: 18 additions & 0 deletions deploy/operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -231,16 +231,22 @@ subjects:
apiVersion: apps/v1
kind: Deployment
metadata:
labels:
webhook-origin: governance-policy-propagator
name: governance-policy-propagator
spec:
replicas: 1
selector:
matchLabels:
name: governance-policy-propagator
webhook-origin: governance-policy-propagator
template:
metadata:
annotations:
kubectl.kubernetes.io/default-container: governance-policy-propagator
labels:
name: governance-policy-propagator
webhook-origin: governance-policy-propagator
spec:
containers:
- args:
Expand All @@ -265,4 +271,16 @@ spec:
- containerPort: 8383
name: http
protocol: TCP
- containerPort: 9443
name: webhook-http
protocol: TCP
volumeMounts:
- mountPath: /tmp/k8s-webhook-server/serving-certs
name: cert
readOnly: true
serviceAccountName: governance-policy-propagator
volumes:
- name: cert
secret:
defaultMode: 420
secretName: propagator-webhook-server-cert
61 changes: 61 additions & 0 deletions deploy/webhook.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
apiVersion: v1
kind: Service
metadata:
name: propagator-webhook-service
namespace: open-cluster-management
spec:
ports:
- port: 443
protocol: TCP
targetPort: 9443
selector:
webhook-origin: governance-policy-propagator
---
apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
name: propagator-webhook-serving-cert
namespace: open-cluster-management
spec:
dnsNames:
- propagator-webhook-service.open-cluster-management.svc
- propagator-webhook-service.open-cluster-management.svc.cluster.local
issuerRef:
kind: Issuer
name: propagator-webhook-selfsigned-issuer
secretName: propagator-webhook-server-cert
---
apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
name: propagator-webhook-selfsigned-issuer
namespace: open-cluster-management
spec:
selfSigned: {}
---
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
annotations:
cert-manager.io/inject-ca-from: open-cluster-management/propagator-webhook-serving-cert
name: propagator-webhook-validating-configuration
webhooks:
- admissionReviewVersions:
- v1
clientConfig:
service:
name: propagator-webhook-service
namespace: open-cluster-management
path: /validate-policy-open-cluster-management-io-v1-policy
failurePolicy: Ignore
name: policy.open-cluster-management.io.webhook
rules:
- apiGroups:
- policy.open-cluster-management.io
apiVersions:
- v1
operations:
- CREATE
resources:
- policies
sideEffects: None
10 changes: 10 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,15 @@ func main() {
var enableLeaderElection bool
var probeAddr string
var keyRotationDays, keyRotationMaxConcurrency, policyMetricsMaxConcurrency, policyStatusMaxConcurrency uint
var enableWebhooks bool

pflag.StringVar(&metricsAddr, "metrics-bind-address", ":8383", "The address the metric endpoint binds to.")
pflag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
pflag.BoolVar(&enableLeaderElection, "leader-elect", true,
"Enable leader election for controller manager. "+
"Enabling this will ensure there is only one active controller manager.")
pflag.BoolVar(&enableWebhooks, "enable-webhooks", true,
"Enable the policy validating webhook")
pflag.UintVar(
&keyRotationDays,
"encryption-key-rotation",
Expand Down Expand Up @@ -301,6 +304,13 @@ func main() {
os.Exit(1)
}

if enableWebhooks {
if err = (&policyv1.Policy{}).SetupWebhookWithManager(mgr); err != nil {
log.Error(err, "unable to create webhook", "webhook", "Policy")
os.Exit(1)
}
}

//+kubebuilder:scaffold:builder

if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
Expand Down
5 changes: 4 additions & 1 deletion main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ import (
// TestRunMain wraps the main() function in order to build a test binary and collection coverage for
// E2E/Integration tests. Controller CLI flags are also passed in here.
func TestRunMain(t *testing.T) {
os.Args = append(os.Args, "--leader-elect=false")
os.Args = append(os.Args,
"--leader-elect=false",
"--enable-webhooks=false",
)

main()
}
Loading

0 comments on commit c93f938

Please sign in to comment.