From 971b4679894b591c2c5547d3d2a59774f321b8c2 Mon Sep 17 00:00:00 2001 From: charles-chenzz <73322208+charles-chenzz@users.noreply.github.com> Date: Sun, 23 Apr 2023 13:40:14 +0800 Subject: [PATCH 1/2] use internal cert to replace cert-manager --- config/default/kustomization.yaml | 58 ++++++++++---------- config/internalcert/kustomization.yaml | 2 + config/internalcert/secret.yaml | 5 ++ config/rbac/role.yaml | 27 ++++++++++ go.mod | 1 + go.sum | 4 ++ main.go | 43 ++++++++++++--- pkg/controllers/jobset_controller.go | 25 ++++----- pkg/util/cert/cert.go | 64 +++++++++++++++++++++++ test/integration/controller/suite_test.go | 3 ++ test/integration/webhook/suite_test.go | 5 +- 11 files changed, 187 insertions(+), 50 deletions(-) create mode 100644 config/internalcert/kustomization.yaml create mode 100644 config/internalcert/secret.yaml create mode 100644 pkg/util/cert/cert.go diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 4e82ca98..f140ba35 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -20,7 +20,7 @@ bases: # crd/kustomization.yaml - ../webhook # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. -- ../certmanager +- ../internalcert # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. #- ../prometheus @@ -39,34 +39,34 @@ patchesStrategicMerge: # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. # Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. # 'CERTMANAGER' needs to be enabled to use ca injection -- webhookcainjection_patch.yaml +#- webhookcainjection_patch.yaml # the following config is for teaching kustomize how to do var substitution -vars: +#vars: # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. -- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR - objref: - kind: Certificate - group: cert-manager.io - version: v1 - name: serving-cert # this name should match the one in certificate.yaml - fieldref: - fieldpath: metadata.namespace -- name: CERTIFICATE_NAME - objref: - kind: Certificate - group: cert-manager.io - version: v1 - name: serving-cert # this name should match the one in certificate.yaml -- name: SERVICE_NAMESPACE # namespace of the service - objref: - kind: Service - version: v1 - name: webhook-service - fieldref: - fieldpath: metadata.namespace -- name: SERVICE_NAME - objref: - kind: Service - version: v1 - name: webhook-service +#- name: CERTIFICATE_NAMESPACE # namespace of the certificate CR +# objref: +# kind: Certificate +# group: cert-manager.io +# version: v1 +# name: serving-cert # this name should match the one in certificate.yaml +# fieldref: +# fieldpath: metadata.namespace +#- name: CERTIFICATE_NAME +# objref: +# kind: Certificate +# group: cert-manager.io +# version: v1 +# name: serving-cert # this name should match the one in certificate.yaml +#- name: SERVICE_NAMESPACE # namespace of the service +# objref: +# kind: Service +# version: v1 +# name: webhook-service +# fieldref: +# fieldpath: metadata.namespace +#- name: SERVICE_NAME +# objref: +# kind: Service +# version: v1 +# name: webhook-service diff --git a/config/internalcert/kustomization.yaml b/config/internalcert/kustomization.yaml new file mode 100644 index 00000000..4409bfc4 --- /dev/null +++ b/config/internalcert/kustomization.yaml @@ -0,0 +1,2 @@ +resources: + - secret.yaml \ No newline at end of file diff --git a/config/internalcert/secret.yaml b/config/internalcert/secret.yaml new file mode 100644 index 00000000..4a889f59 --- /dev/null +++ b/config/internalcert/secret.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +kind: Secret +metadata: + name: webhook-server-cert + namespace: system \ No newline at end of file diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 3822839f..0d0a5d27 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -25,6 +25,33 @@ rules: - patch - update - watch +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - update + - watch +- apiGroups: + - admissionregistration.k8s.io + resources: + - mutatingwebhookconfigurations + verbs: + - get + - list + - update + - watch +- apiGroups: + - admissionregistration.k8s.io + resources: + - validatingwebhookconfigurations + verbs: + - get + - list + - update + - watch - apiGroups: - batch resources: diff --git a/go.mod b/go.mod index a1ba107a..796cfbbc 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/google/go-cmp v0.5.9 github.com/onsi/ginkgo/v2 v2.9.2 github.com/onsi/gomega v1.27.6 + github.com/open-policy-agent/cert-controller v0.7.0 k8s.io/api v0.26.4 k8s.io/apimachinery v0.26.4 k8s.io/client-go v0.26.4 diff --git a/go.sum b/go.sum index 211bd398..b2100677 100644 --- a/go.sum +++ b/go.sum @@ -129,6 +129,9 @@ github.com/onsi/ginkgo/v2 v2.9.2 h1:BA2GMJOtfGAfagzYtrAlufIP0lq6QERkFmHLMLPwFSU= github.com/onsi/ginkgo/v2 v2.9.2/go.mod h1:WHcJJG2dIlcCqVfBAwUCrJxSPFb6v4azBwgxeMeDuts= github.com/onsi/gomega v1.27.6 h1:ENqfyGeS5AX/rlXDd/ETokDz93u0YufY1Pgxuy/PvWE= github.com/onsi/gomega v1.27.6/go.mod h1:PIQNjfQwkP3aQAH7lf7j87O/5FiNr+ZR8+ipb+qQlhg= +github.com/open-policy-agent/cert-controller v0.7.0 h1:5ggZjSQJ1YgkT+ngNAGBGHaOYfGuYq97IrNldchCoHI= +github.com/open-policy-agent/cert-controller v0.7.0/go.mod h1:Dkkdcr1BeSUig/62dYLqgvx3lIN0XtEikKTGW1lKSQo= +github.com/open-policy-agent/frameworks/constraint v0.0.0-20230201235642-777dc99a6669 h1:vKt4PhZXBxYHeLujYraNVpkoILQ/NISiifzaq1DkMXk= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -315,6 +318,7 @@ k8s.io/component-base v0.26.3 h1:oC0WMK/ggcbGDTkdcqefI4wIZRYdK3JySx9/HADpV0g= k8s.io/component-base v0.26.3/go.mod h1:5kj1kZYwSC6ZstHJN7oHBqcJC6yyn41eR+Sqa/mQc8E= k8s.io/klog/v2 v2.90.1 h1:m4bYOKall2MmOiRaR1J+We67Do7vm9KiQVlT96lnHUw= k8s.io/klog/v2 v2.90.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= +k8s.io/kube-aggregator v0.23.2 h1:6CoZZqNdFc9benrgSJJ0GQGgFtKjI0y3UwlBbioXtc8= k8s.io/kube-openapi v0.0.0-20230327201221-f5883ff37f0c h1:EFfsozyzZ/pggw5qNx7ftTVZdp7WZl+3ih89GEjYEK8= k8s.io/kube-openapi v0.0.0-20230327201221-f5883ff37f0c/go.mod h1:byini6yhqGC14c3ebc/QwanvYwhuMWF6yz2F8uwW8eg= k8s.io/utils v0.0.0-20230313181309-38a27ef9d749 h1:xMMXJlJbsU8w3V5N2FLDQ8YgU8s1EoULdbQBcAeNJkY= diff --git a/main.go b/main.go index 551f555a..15b222ac 100644 --- a/main.go +++ b/main.go @@ -19,6 +19,7 @@ package main import ( "flag" "os" + "sigs.k8s.io/jobset/pkg/util/cert" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. @@ -88,16 +89,46 @@ func main() { setupLog.Error(err, "unable to start manager") os.Exit(1) } + + certsReady := make(chan struct{}) + if err = cert.CertsManager(mgr, certsReady); err != nil { + setupLog.Error(err, "unable to setup cert rotation") + os.Exit(1) + } + + setupIndexes(mgr) + + go setupControllers(mgr, certsReady) + + setupHealthzAndReadyzCheck(mgr) + + setupLog.Info("starting manager") + if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { + setupLog.Error(err, "problem running manager") + os.Exit(1) + } +} + +func setupControllers(mgr ctrl.Manager, certsReady chan struct{}) { + // wait until the cert ready, until then we set up controllers + setupLog.Info("waiting for the cert generation to complete") + <-certsReady + setupLog.Info("certs ready") + jobSetController := controllers.NewJobSetReconciler(mgr.GetClient(), mgr.GetScheme(), mgr.GetEventRecorderFor("jobset")) - if err = jobSetController.SetupWithManager(mgr); err != nil { + if err := jobSetController.SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "JobSet") os.Exit(1) } - if err = (&jobset.JobSet{}).SetupWebhookWithManager(mgr); err != nil { + if err := (&jobset.JobSet{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "JobSet") os.Exit(1) } //+kubebuilder:scaffold:builder +} + +func setupHealthzAndReadyzCheck(mgr ctrl.Manager) { + defer setupLog.Info("both healthz and readyz check are finished and configured") if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up health check") @@ -107,10 +138,10 @@ func main() { setupLog.Error(err, "unable to set up ready check") os.Exit(1) } +} - setupLog.Info("starting manager") - if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { - setupLog.Error(err, "problem running manager") - os.Exit(1) +func setupIndexes(mgr ctrl.Manager) { + if err := controllers.SetupIndexes(mgr.GetFieldIndexer()); err != nil { + setupLog.Error(err, "unable to setup indexes") } } diff --git a/pkg/controllers/jobset_controller.go b/pkg/controllers/jobset_controller.go index c4cbabcf..d61b2573 100644 --- a/pkg/controllers/jobset_controller.go +++ b/pkg/controllers/jobset_controller.go @@ -134,10 +134,16 @@ func (r *JobSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // SetupWithManager sets up the controller with the Manager. func (r *JobSetReconciler) SetupWithManager(mgr ctrl.Manager) error { - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &batchv1.Job{}, jobOwnerKey, func(rawObj client.Object) []string { - // grab the job object, extract the owner... - job := rawObj.(*batchv1.Job) - owner := metav1.GetControllerOf(job) + return ctrl.NewControllerManagedBy(mgr). + For(&jobset.JobSet{}). + Owns(&batchv1.Job{}). + Complete(r) +} + +func SetupIndexes(indexer client.FieldIndexer) error { + return indexer.IndexField(context.Background(), &batchv1.Job{}, jobOwnerKey, func(obj client.Object) []string { + o := obj.(*batchv1.Job) + owner := metav1.GetControllerOf(o) if owner == nil { return nil } @@ -145,17 +151,8 @@ func (r *JobSetReconciler) SetupWithManager(mgr ctrl.Manager) error { if owner.APIVersion != apiGVStr || owner.Kind != "JobSet" { return nil } - - // ...and if so, return it return []string{owner.Name} - }); err != nil { - return err - } - - return ctrl.NewControllerManagedBy(mgr). - For(&jobset.JobSet{}). - Owns(&batchv1.Job{}). - Complete(r) + }) } // getChildJobs gets jobs owned by the JobSet then categorizes them by status (active, successful, failed). diff --git a/pkg/util/cert/cert.go b/pkg/util/cert/cert.go new file mode 100644 index 00000000..0903a792 --- /dev/null +++ b/pkg/util/cert/cert.go @@ -0,0 +1,64 @@ +/* +Copyright 2023 The Kubernetes Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cert + +import ( + "fmt" + cert "github.com/open-policy-agent/cert-controller/pkg/rotator" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" +) + +const ( + serviceName = "jobset-webhook-service" + secretName = "jobset-webhook-server-cert" + secretNamespace = "jobset-system" + certDir = "/tmp/k8s-webhook-server/serving-certs" + validateWebhookConfName = "jobset-validating-webhook-configuration" + mutatingWebhookConfName = "jobset-mutating-webhook-configuration" + caName = "jobset-ca" + caOrg = "jobset" +) + +// dnsName is the format of ..svc +var dnsName = fmt.Sprintf("%s.%s.svc", serviceName, secretNamespace) + +//+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;update +//+kubebuilder:rbac:groups="admissionregistration.k8s.io",resources=mutatingwebhookconfigurations,verbs=get;list;watch;update +//+kubebuilder:rbac:groups="admissionregistration.k8s.io",resources=validatingwebhookconfigurations,verbs=get;list;watch;update + +// CertsManager creates certs for webhooks. +func CertsManager(mgr ctrl.Manager, setupFinish chan struct{}) error { + return cert.AddRotator(mgr, &cert.CertRotator{ + SecretKey: types.NamespacedName{ + Namespace: secretNamespace, + Name: secretName, + }, + CertDir: certDir, + CAName: caName, + CAOrganization: caOrg, + DNSName: dnsName, + IsReady: setupFinish, + Webhooks: []cert.WebhookInfo{ + { + Type: cert.Validating, + Name: validateWebhookConfName, + }, + { + Type: cert.Mutating, + Name: mutatingWebhookConfName, + }, + }, + }) +} diff --git a/test/integration/controller/suite_test.go b/test/integration/controller/suite_test.go index bd770f8a..781afb6e 100644 --- a/test/integration/controller/suite_test.go +++ b/test/integration/controller/suite_test.go @@ -84,6 +84,9 @@ var _ = BeforeSuite(func() { Expect(err).ToNot(HaveOccurred()) jobSetController := controllers.NewJobSetReconciler(k8sManager.GetClient(), k8sManager.GetScheme(), k8sManager.GetEventRecorderFor("jobset")) + err = controllers.SetupIndexes(k8sManager.GetFieldIndexer()) + Expect(err).ToNot(HaveOccurred()) + err = jobSetController.SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) diff --git a/test/integration/webhook/suite_test.go b/test/integration/webhook/suite_test.go index 3224b4c2..42d797bd 100644 --- a/test/integration/webhook/suite_test.go +++ b/test/integration/webhook/suite_test.go @@ -35,8 +35,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" - jobset "sigs.k8s.io/jobset/api/v1alpha1" + "sigs.k8s.io/jobset/pkg/controllers" ) // These tests use Ginkgo (BDD-style Go testing framework). Refer to @@ -99,6 +99,9 @@ var _ = BeforeSuite(func() { }) Expect(err).NotTo(HaveOccurred()) + err = controllers.SetupIndexes(mgr.GetFieldIndexer()) + Expect(err).NotTo(HaveOccurred()) + err = (&jobset.JobSet{}).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) From 96a8a6c2398bf1e5e3cf109b5d349dc863365898 Mon Sep 17 00:00:00 2001 From: charles-chenzz Date: Mon, 24 Apr 2023 11:05:39 +0800 Subject: [PATCH 2/2] update README.md and Dockerfile. --- Dockerfile | 1 + main.go | 18 +++++++++--------- pkg/util/cert/cert.go | 1 + test/integration/webhook/suite_test.go | 1 + 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Dockerfile b/Dockerfile index c8b7bc85..98f94d19 100644 --- a/Dockerfile +++ b/Dockerfile @@ -15,6 +15,7 @@ RUN go mod download COPY main.go main.go COPY api/ api/ COPY pkg/controllers/ pkg/controllers/ +COPY pkg/util/cert/ pkg/util/cert/ # Build # the GOARCH has not a default value to allow the binary be built according to the host where the command diff --git a/main.go b/main.go index 15b222ac..951c3f24 100644 --- a/main.go +++ b/main.go @@ -19,7 +19,6 @@ package main import ( "flag" "os" - "sigs.k8s.io/jobset/pkg/util/cert" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. @@ -34,6 +33,7 @@ import ( jobset "sigs.k8s.io/jobset/api/v1alpha1" "sigs.k8s.io/jobset/pkg/controllers" + "sigs.k8s.io/jobset/pkg/util/cert" //+kubebuilder:scaffold:imports ) @@ -96,8 +96,13 @@ func main() { os.Exit(1) } - setupIndexes(mgr) + if err := controllers.SetupIndexes(mgr.GetFieldIndexer()); err != nil { + setupLog.Error(err, "unable to setup indexes") + } + // Cert won't be ready until manager starts, so start a goroutine here which + // will block until the cert is ready before setting up the controllers. + // Controllers who register after manager starts will start directly. go setupControllers(mgr, certsReady) setupHealthzAndReadyzCheck(mgr) @@ -110,7 +115,8 @@ func main() { } func setupControllers(mgr ctrl.Manager, certsReady chan struct{}) { - // wait until the cert ready, until then we set up controllers + // The controllers won't work until the webhooks are operating, + // and the webhook won't work until the certs are all in places. setupLog.Info("waiting for the cert generation to complete") <-certsReady setupLog.Info("certs ready") @@ -139,9 +145,3 @@ func setupHealthzAndReadyzCheck(mgr ctrl.Manager) { os.Exit(1) } } - -func setupIndexes(mgr ctrl.Manager) { - if err := controllers.SetupIndexes(mgr.GetFieldIndexer()); err != nil { - setupLog.Error(err, "unable to setup indexes") - } -} diff --git a/pkg/util/cert/cert.go b/pkg/util/cert/cert.go index 0903a792..ab767b64 100644 --- a/pkg/util/cert/cert.go +++ b/pkg/util/cert/cert.go @@ -15,6 +15,7 @@ package cert import ( "fmt" + cert "github.com/open-policy-agent/cert-controller/pkg/rotator" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" diff --git a/test/integration/webhook/suite_test.go b/test/integration/webhook/suite_test.go index 42d797bd..96cdd3fe 100644 --- a/test/integration/webhook/suite_test.go +++ b/test/integration/webhook/suite_test.go @@ -35,6 +35,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + jobset "sigs.k8s.io/jobset/api/v1alpha1" "sigs.k8s.io/jobset/pkg/controllers" )