From 927cef3c22987cf54af819a87b145e563e021a8c Mon Sep 17 00:00:00 2001 From: Tim Ramlot <42113979+inteon@users.noreply.github.com> Date: Wed, 26 Apr 2023 17:04:11 +0200 Subject: [PATCH] switch to SSA for cainjector Co-authored-by: joshvanl Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com> --- cmd/cainjector/app/start.go | 32 ++-- .../templates/cainjector-rbac.yaml | 6 +- design/20220118.server-side-apply.md | 2 +- make/e2e-setup.mk | 2 +- pkg/controller/cainjector/injectables.go | 138 ++++++++++++++++++ pkg/controller/cainjector/reconciler.go | 18 ++- pkg/controller/cainjector/setup.go | 9 +- test/e2e/suite/serving/cainjector.go | 2 +- 8 files changed, 182 insertions(+), 27 deletions(-) diff --git a/cmd/cainjector/app/start.go b/cmd/cainjector/app/start.go index 45f435220..8a6a43cca 100644 --- a/cmd/cainjector/app/start.go +++ b/cmd/cainjector/app/start.go @@ -143,7 +143,7 @@ func NewCommandStartInjectorController(ctx context.Context, out, errOut io.Write o := NewInjectorControllerOptions(out, errOut) cmd := &cobra.Command{ - Use: "ca-injector", + Use: "cainjector", Short: fmt.Sprintf("CA Injection Controller for Kubernetes (%s) (%s)", util.AppVersion, util.AppGitCommit), Long: ` cert-manager CA injector is a Kubernetes addon to automate the injection of CA data into @@ -155,7 +155,7 @@ servers and webhook servers.`, // TODO: Refactor this function from this package RunE: func(cmd *cobra.Command, args []string) error { - o.log = logf.Log.WithName("ca-injector") + o.log = logf.Log.WithName("cainjector") logf.V(logf.InfoLevel).InfoS("starting", "version", util.AppVersion, "revision", util.AppGitCommit) return o.RunInjectorController(ctx) @@ -169,19 +169,21 @@ servers and webhook servers.`, } func (o InjectorControllerOptions) RunInjectorController(ctx context.Context) error { - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ - Scheme: api.Scheme, - Namespace: o.Namespace, - LeaderElection: o.LeaderElect, - LeaderElectionNamespace: o.LeaderElectionNamespace, - LeaderElectionID: "cert-manager-cainjector-leader-election", - LeaderElectionReleaseOnCancel: true, - LeaderElectionResourceLock: resourcelock.LeasesResourceLock, - LeaseDuration: &o.LeaseDuration, - RenewDeadline: &o.RenewDeadline, - RetryPeriod: &o.RetryPeriod, - MetricsBindAddress: "0", - }) + mgr, err := ctrl.NewManager( + util.RestConfigWithUserAgent(ctrl.GetConfigOrDie(), "cainjector"), + ctrl.Options{ + Scheme: api.Scheme, + Namespace: o.Namespace, + LeaderElection: o.LeaderElect, + LeaderElectionNamespace: o.LeaderElectionNamespace, + LeaderElectionID: "cert-manager-cainjector-leader-election", + LeaderElectionReleaseOnCancel: true, + LeaderElectionResourceLock: resourcelock.LeasesResourceLock, + LeaseDuration: &o.LeaseDuration, + RenewDeadline: &o.RenewDeadline, + RetryPeriod: &o.RetryPeriod, + MetricsBindAddress: "0", + }) if err != nil { return fmt.Errorf("error creating manager: %v", err) } diff --git a/deploy/charts/cert-manager/templates/cainjector-rbac.yaml b/deploy/charts/cert-manager/templates/cainjector-rbac.yaml index 0393f92be..2aa59eee9 100644 --- a/deploy/charts/cert-manager/templates/cainjector-rbac.yaml +++ b/deploy/charts/cert-manager/templates/cainjector-rbac.yaml @@ -22,13 +22,13 @@ rules: verbs: ["get", "create", "update", "patch"] - apiGroups: ["admissionregistration.k8s.io"] resources: ["validatingwebhookconfigurations", "mutatingwebhookconfigurations"] - verbs: ["get", "list", "watch", "update"] + verbs: ["get", "list", "watch", "update", "patch"] - apiGroups: ["apiregistration.k8s.io"] resources: ["apiservices"] - verbs: ["get", "list", "watch", "update"] + verbs: ["get", "list", "watch", "update", "patch"] - apiGroups: ["apiextensions.k8s.io"] resources: ["customresourcedefinitions"] - verbs: ["get", "list", "watch", "update"] + verbs: ["get", "list", "watch", "update", "patch"] --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding diff --git a/design/20220118.server-side-apply.md b/design/20220118.server-side-apply.md index f4f238c88..f98f7d25b 100644 --- a/design/20220118.server-side-apply.md +++ b/design/20220118.server-side-apply.md @@ -112,7 +112,7 @@ cert-manager-certificates-[issuing,trigger,keymanager,readiness] cert-manager-certificaterequests-[acme,approver,ca,selfsigned,vault,venafi] cert-manager-clusterissuers-[acme,ca,selfsigned,vault,venafi] cert-manager-issuers-[acme,ca,selfsigned,vault,venafi] -cert-manager-cainjector # base field manager of cert-manager ca-injector +cert-manager-cainjector # base field manager of cert-manager cainjector cert-manager-webhook # base field manager of cert-manager webhook cert-manager-cmctl ``` diff --git a/make/e2e-setup.mk b/make/e2e-setup.mk index ceab12bce..2c77dc181 100644 --- a/make/e2e-setup.mk +++ b/make/e2e-setup.mk @@ -237,7 +237,7 @@ comma = , # for "--set featureGates". That's why we have "\$(comma)". feature_gates_controller := $(subst $(space),\$(comma),$(filter AllAlpha=% AllBeta=% AdditionalCertificateOutputFormats=% ValidateCAA=% ExperimentalCertificateSigningRequestControllers=% ExperimentalGatewayAPISupport=% ServerSideApply=% LiteralCertificateSubject=% UseCertificateRequestBasicConstraints=% SecretsFilteredCaching=%, $(subst $(comma),$(space),$(FEATURE_GATES)))) feature_gates_webhook := $(subst $(space),\$(comma),$(filter AllAlpha=% AllBeta=% AdditionalCertificateOutputFormats=% LiteralCertificateSubject=%, $(subst $(comma),$(space),$(FEATURE_GATES)))) -feature_gates_cainjector := $(subst $(space),\$(comma),$(filter AllAlpha=% AllBeta=%, $(subst $(comma),$(space),$(FEATURE_GATES)))) +feature_gates_cainjector := $(subst $(space),\$(comma),$(filter AllAlpha=% AllBeta=% ServerSideApply=%, $(subst $(comma),$(space),$(FEATURE_GATES)))) # Install cert-manager with E2E specific images and deployment settings. # The values.best-practice.yaml file is applied for compliance with the diff --git a/pkg/controller/cainjector/injectables.go b/pkg/controller/cainjector/injectables.go index e76504bc4..1f1537a91 100644 --- a/pkg/controller/cainjector/injectables.go +++ b/pkg/controller/cainjector/injectables.go @@ -17,8 +17,13 @@ limitations under the License. package cainjector import ( + "encoding/json" + admissionreg "k8s.io/api/admissionregistration/v1" apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/types" + applyadmissionreg "k8s.io/client-go/applyconfigurations/admissionregistration/v1" + applymetav1 "k8s.io/client-go/applyconfigurations/meta/v1" apireg "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -61,12 +66,38 @@ type InjectTarget interface { // It should be a pointer suitable for mutation. AsObject() client.Object + // AsApplyObject returns this injectable as an object that only contains + // fields which are managed by the cainjector (CA Data) and immutable fields + // that must be present in Apply calls; intended for use for Apply Patch + // calls. + AsApplyObject() (client.Object, client.Patch) + // SetCA sets the CA of this target to the given certificate data (in the standard // PEM format used across Kubernetes). In cases where multiple CA fields exist per // target (like admission webhook configs), all CAs are set to the given value. SetCA(data []byte) } +type ssaPatch struct { + patch []byte + err error +} + +func newSSAPatch(patch interface{}) *ssaPatch { + jsonPatch, err := json.Marshal(patch) + return &ssaPatch{patch: jsonPatch, err: err} +} + +func (p *ssaPatch) Type() types.PatchType { + return types.ApplyPatchType +} + +func (p *ssaPatch) Data(obj client.Object) ([]byte, error) { + return p.patch, nil +} + +var _ client.Patch = &ssaPatch{} + // mutatingWebhookTarget knows how to set CA data for all the webhooks // in a mutatingWebhookConfiguration. type mutatingWebhookTarget struct { @@ -76,12 +107,32 @@ type mutatingWebhookTarget struct { func (t *mutatingWebhookTarget) AsObject() client.Object { return &t.obj } + func (t *mutatingWebhookTarget) SetCA(data []byte) { for ind := range t.obj.Webhooks { t.obj.Webhooks[ind].ClientConfig.CABundle = data } } +func (t *mutatingWebhookTarget) AsApplyObject() (client.Object, client.Patch) { + patch := applyadmissionreg.MutatingWebhookConfiguration(t.obj.Name) + + for i := range t.obj.Webhooks { + patch = patch.WithWebhooks( + applyadmissionreg. + MutatingWebhook(). + WithName(t.obj.Webhooks[i].Name). // Name is used as slice key. + WithClientConfig( + &applyadmissionreg.WebhookClientConfigApplyConfiguration{ + CABundle: t.obj.Webhooks[i].ClientConfig.CABundle, + }, + ), + ) + } + + return &t.obj, newSSAPatch(patch) +} + // validatingWebhookTarget knows how to set CA data for all the webhooks // in a validatingWebhookConfiguration. type validatingWebhookTarget struct { @@ -98,6 +149,25 @@ func (t *validatingWebhookTarget) SetCA(data []byte) { } } +func (t *validatingWebhookTarget) AsApplyObject() (client.Object, client.Patch) { + patch := applyadmissionreg.ValidatingWebhookConfiguration(t.obj.Name) + + for i := range t.obj.Webhooks { + patch = patch.WithWebhooks( + applyadmissionreg. + ValidatingWebhook(). + WithName(t.obj.Webhooks[i].Name). // Name is used as slice key. + WithClientConfig( + &applyadmissionreg.WebhookClientConfigApplyConfiguration{ + CABundle: t.obj.Webhooks[i].ClientConfig.CABundle, + }, + ), + ) + } + + return &t.obj, newSSAPatch(patch) +} + // apiServiceTarget knows how to set CA data for the CA bundle in // the APIService spec. type apiServiceTarget struct { @@ -112,6 +182,31 @@ func (t *apiServiceTarget) SetCA(data []byte) { t.obj.Spec.CABundle = data } +type apiServiceTargetPatch struct { + applymetav1.TypeMetaApplyConfiguration `json:",inline"` + *applymetav1.ObjectMetaApplyConfiguration `json:"metadata,omitempty"` + Spec *apiServiceTargetSpecPatch `json:"spec,omitempty"` +} + +type apiServiceTargetSpecPatch struct { + CABundle []byte `json:"caBundle,omitempty"` +} + +func (t *apiServiceTarget) AsApplyObject() (client.Object, client.Patch) { + return &t.obj, newSSAPatch(&apiServiceTargetPatch{ + TypeMetaApplyConfiguration: *applymetav1. + TypeMeta(). + WithAPIVersion(apireg.SchemeGroupVersion.String()). + WithKind("APIService"), + ObjectMetaApplyConfiguration: applymetav1. + ObjectMeta(). + WithName(t.obj.Name), + Spec: &apiServiceTargetSpecPatch{ + CABundle: t.obj.Spec.CABundle, + }, + }) +} + // crdConversionTarget knows how to set CA data for the conversion webhook in CRDs type crdConversionTarget struct { obj apiext.CustomResourceDefinition @@ -133,3 +228,46 @@ func (t *crdConversionTarget) SetCA(data []byte) { } t.obj.Spec.Conversion.Webhook.ClientConfig.CABundle = data } + +type customResourceDefinitionPatch struct { + applymetav1.TypeMetaApplyConfiguration `json:",inline"` + *applymetav1.ObjectMetaApplyConfiguration `json:"metadata,omitempty"` + Spec *customResourceDefinitionSpecPatch `json:"spec,omitempty"` +} + +type customResourceDefinitionSpecPatch struct { + Conversion *customResourceConversionPatch `json:"conversion,omitempty"` +} + +type customResourceConversionPatch struct { + Webhook *customResourceWebhookConversionPatch `json:"webhook,omitempty"` +} + +type customResourceWebhookConversionPatch struct { + ClientConfig *customResourceWebhookClientConfigPatch `json:"clientConfig,omitempty"` +} + +type customResourceWebhookClientConfigPatch struct { + CABundle []byte `json:"caBundle,omitempty"` +} + +func (t *crdConversionTarget) AsApplyObject() (client.Object, client.Patch) { + return &t.obj, newSSAPatch(&customResourceDefinitionPatch{ + TypeMetaApplyConfiguration: *applymetav1. + TypeMeta(). + WithAPIVersion(apiext.SchemeGroupVersion.String()). + WithKind("CustomResourceDefinition"), + ObjectMetaApplyConfiguration: applymetav1. + ObjectMeta(). + WithName(t.obj.Name), + Spec: &customResourceDefinitionSpecPatch{ + Conversion: &customResourceConversionPatch{ + Webhook: &customResourceWebhookConversionPatch{ + ClientConfig: &customResourceWebhookClientConfigPatch{ + CABundle: t.obj.Spec.Conversion.Webhook.ClientConfig.CABundle, + }, + }, + }, + }, + }) +} diff --git a/pkg/controller/cainjector/reconciler.go b/pkg/controller/cainjector/reconciler.go index dd158a5e3..66bc5841b 100644 --- a/pkg/controller/cainjector/reconciler.go +++ b/pkg/controller/cainjector/reconciler.go @@ -21,12 +21,15 @@ import ( "fmt" "strings" + "github.com/cert-manager/cert-manager/internal/controller/feature" + utilfeature "github.com/cert-manager/cert-manager/pkg/util/feature" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -57,6 +60,9 @@ type reconciler struct { // if set, the reconciler is namespace scoped namespace string + // fieldManager is the manager name used for the Apply operations. + fieldManager string + resourceName string // just used for logging } @@ -117,10 +123,20 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu target.SetCA(caData) // actually update with injected CA data - if err := r.Client.Update(ctx, target.AsObject()); err != nil { + if utilfeature.DefaultFeatureGate.Enabled(feature.ServerSideApply) { + obj, patch := target.AsApplyObject() + err = r.Client.Patch(ctx, obj, patch, &client.PatchOptions{ + Force: pointer.Bool(true), FieldManager: r.fieldManager, + }) + } else { + err = r.Client.Update(ctx, target.AsObject()) + } + + if err != nil { log.Error(err, "unable to update target object with new CA data") return ctrl.Result{}, err } + log.V(logf.InfoLevel).Info("Updated object") return ctrl.Result{}, nil diff --git a/pkg/controller/cainjector/setup.go b/pkg/controller/cainjector/setup.go index 65a37fea9..e2bad4d1a 100644 --- a/pkg/controller/cainjector/setup.go +++ b/pkg/controller/cainjector/setup.go @@ -35,6 +35,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + "github.com/cert-manager/cert-manager/pkg/util" ) // this file contains the logic to set up the different reconcile loops run by cainjector @@ -90,8 +91,6 @@ var ( listType: &apiext.CustomResourceDefinitionList{}, objType: &apiext.CustomResourceDefinition{}, } - - injectorSetups = []setup{MutatingWebhookSetup, ValidatingWebhookSetup, APIServiceSetup, CRDSetup} ) // registerAllInjectors registers all injectors and based on the @@ -134,6 +133,7 @@ func RegisterAllInjectors(ctx context.Context, mgr ctrl.Manager, opts SetupOptio cds, kds, }, + fieldManager: util.PrefixFromUserAgent(mgr.GetConfig().UserAgent), } // Index injectable with a new field. If the injectable's CA is @@ -196,9 +196,8 @@ func RegisterAllInjectors(ctx context.Context, mgr ctrl.Manager, opts SetupOptio toInjectable: buildCertToInjectableFunc(setup.listType, setup.resourceName), }).Map)) } - err := b.Complete(r) - if err != nil { - err = fmt.Errorf("error registering controller for %s: %w", setup.objType.GetName(), err) + if err := b.Complete(r); err != nil { + return fmt.Errorf("error registering controller for %s: %w", setup.objType.GetName(), err) } } return nil diff --git a/test/e2e/suite/serving/cainjector.go b/test/e2e/suite/serving/cainjector.go index 0497d3a4c..5a0682668 100644 --- a/test/e2e/suite/serving/cainjector.go +++ b/test/e2e/suite/serving/cainjector.go @@ -49,7 +49,7 @@ type injectableTest struct { } var _ = framework.CertManagerDescribe("CA Injector", func() { - f := framework.NewDefaultFramework("ca-injector") + f := framework.NewDefaultFramework("cainjector") issuerName := "inject-cert-issuer" secretName := "serving-certs-data"