Merge pull request #359 from SgtCoDFish/dynamic-istiod

Add ability to dynamically configure istiod cert

cmd/app/app.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 
+	cmapi "github.com/cert-manager/cert-manager/pkg/api"
 	"github.com/spf13/cobra"
 	"k8s.io/apimachinery/pkg/api/validation"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -34,6 +35,7 @@ import (
 	"github.com/cert-manager/istio-csr/cmd/app/options"
 	"github.com/cert-manager/istio-csr/pkg/certmanager"
 	"github.com/cert-manager/istio-csr/pkg/controller"
+	"github.com/cert-manager/istio-csr/pkg/istiodcert"
 	"github.com/cert-manager/istio-csr/pkg/server"
 	"github.com/cert-manager/istio-csr/pkg/tls"
 )
@@ -58,6 +60,7 @@ func NewCommand(ctx context.Context) *cobra.Command {
 			if errs := validation.ValidateAnnotations(opts.CertManager.AdditionalAnnotations, field.NewPath("certificate-request-additional-annotations")); len(errs) > 0 {
 				return errs.ToAggregate()
 			}
+
 			cm, err := certmanager.New(opts.Logr, opts.RestConfig, opts.CertManager)
 			if err != nil {
 				return fmt.Errorf("failed to initialise cert-manager manager: %w", err)
@@ -68,6 +71,10 @@ func NewCommand(ctx context.Context) *cobra.Command {
 				return fmt.Errorf("failed to add kubernetes scheme: %s", err)
 			}
 
+			if err := cmapi.AddToScheme(intscheme); err != nil {
+				return fmt.Errorf("failed to add cert-manager scheme: %s", err)
+			}
+
 			cl, err := kubernetes.NewForConfig(opts.RestConfig)
 			if err != nil {
 				return fmt.Errorf("error creating kubernetes client: %s", err.Error())
@@ -101,14 +108,26 @@ func NewCommand(ctx context.Context) *cobra.Command {
 				return fmt.Errorf("failed to create manager: %w", err)
 			}
 
+			if opts.IstiodCert.Enabled {
+				istiodCertController, err := istiodcert.New(opts.Logr.WithName("istiod-dynamic"), opts.RestConfig, opts.IstiodCert, cm, opts.TLS.TrustDomain)
+				if err != nil {
+					return fmt.Errorf("failed to create dynamic istiod certificate provisioner: %s", err)
+				}
+
+				err = istiodCertController.AddControllersToManager(mgr)
+				if err != nil {
+					return fmt.Errorf("failed to add dynamic istiod cert controllers: %w", err)
+				}
+			}
+
 			if opts.CertManager.HasRuntimeConfiguration() {
 				if err := mgr.Add(cm.RuntimeConfigurationWatcher(ctx)); err != nil {
 					return fmt.Errorf("failed to add runtime configuration watcher as runnable: %w", err)
 				}
 			}
 
 			// Create a new TLS provider for the serving certificate and private key.
-			tls, err := tls.NewProvider(opts.Logr, cm, opts.TLS)
+			tls, err := tls.NewProvider(opts.Logr, cm, opts.TLS, cm)
 			if err != nil {
 				return fmt.Errorf("failed to create tls provider: %w", err)
 			}

cmd/app/options/options.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/klog/v2"
 
 	"github.com/cert-manager/istio-csr/pkg/certmanager"
+	"github.com/cert-manager/istio-csr/pkg/istiodcert"
 	"github.com/cert-manager/istio-csr/pkg/server"
 	"github.com/cert-manager/istio-csr/pkg/tls"
 
@@ -64,6 +65,7 @@ type Options struct {
 	CertManager certmanager.Options
 	TLS         tls.Options
 	Server      server.Options
+	IstiodCert  istiodcert.Options
 }
 
 // OptionsController is the Controller specific options
@@ -155,6 +157,11 @@ func (o *Options) Complete() error {
 		log.Info("WARNING: --preserve-certificate-requests is enabled. Do not enable this option in production, or environments with any non-trivial number of workloads for an extended period of time. Doing so will balloon the resource consumption of ETCD, the API server, and istio-csr, leading to errors and slowdown. This option is intended for debugging purposes only, for limited periods of time.")
 	}
 
+	err = o.IstiodCert.Validate()
+	if err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -170,6 +177,8 @@ func (o *Options) addFlags(cmd *cobra.Command) {
 	o.addControllerFlags(nfs.FlagSet("controller"))
 	o.addAdditionalAnnotationsFlags(nfs.FlagSet("additional-annotations"))
 
+	istiodcert.AddFlags(&o.IstiodCert, nfs.FlagSet("istiod-cert"))
+
 	usageFmt := "Usage:\n  %s\n"
 	cmd.SetUsageFunc(func(cmd *cobra.Command) error {
 		fmt.Fprintf(cmd.OutOrStderr(), usageFmt, cmd.UseLine())

deploy/charts/istio-csr/README.md

@@ -250,13 +250,14 @@ An optional file location to a PEM encoded root CA that the root CA. ConfigMap i
 Requested duration of gRPC serving certificate. Will be automatically renewed.  
 Based on NIST 800-204A recommendations (SM-DR13).  
 https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-204A.pdf
-#### **app.tls.istiodCertificateEnable** ~ `bool`
+#### **app.tls.istiodCertificateEnable** ~ `boolean,string,null`
 > Default value:
 > ```yaml
 > true
 > ```
 
-Create the default certificate as part of install.
+If true, create the istiod certificate using a cert-manager certificate as part of the install. If set to "dynamic", will create the cert dynamically when istio-csr pods start up. If false, no cert is created.
+
 #### **app.tls.istiodCertificateDuration** ~ `string`
 > Default value:
 > ```yaml

deploy/charts/istio-csr/templates/certificate.yaml

@@ -1,4 +1,4 @@
-{{- if .Values.app.tls.istiodCertificateEnable }}
+{{- if eq (toString .Values.app.tls.istiodCertificateEnable) "true" }}
 apiVersion: cert-manager.io/v1
 kind: Certificate
 metadata:

deploy/charts/istio-csr/templates/clusterrole.yaml

@@ -21,3 +21,13 @@ rules:
   - "tokenreviews"
   verbs:
   - "create"
+{{- if eq (toString .Values.app.tls.istiodCertificateEnable) "dynamic" }}
+- apiGroups:
+  - "cert-manager.io"
+  resources:
+  - "certificates"
+  verbs:
+  - "list"
+  - "get"
+  - "watch"
+{{- end }}

deploy/charts/istio-csr/templates/deployment.yaml

@@ -94,6 +94,17 @@ spec:
 
           - "--runtime-issuance-config-map-name={{.Values.app.runtimeIssuanceConfigMap}}"
           - "--runtime-issuance-config-map-namespace={{.Release.Namespace}}"
+
+          # dynamic istiod cert
+          - "--istiod-cert-enabled={{ eq (toString .Values.app.tls.istiodCertificateEnable) "dynamic" }}"
+          - "--istiod-cert-name=istiod-dynamic"
+          - "--istiod-cert-namespace={{ .Values.app.istio.namespace }}"
+          - "--istiod-cert-duration={{ .Values.app.tls.istiodCertificateDuration }}"
+          - "--istiod-cert-renew-before={{ .Values.app.tls.istiodCertificateRenewBefore }}"
+          - "--istiod-cert-key-algorithm={{ default .Values.app.server.serving.signatureAlgorithm .Values.app.tls.istiodPrivateKeyAlgorithm }}"
+          - "--istiod-cert-key-size={{ .Values.app.tls.istiodPrivateKeySize }}"
+          - "--istiod-cert-additional-dns-names={{ join "," .Values.app.tls.istiodAdditionalDNSNames }}"
+          - "--istiod-cert-istio-revisions={{ join "," .Values.app.istio.revisions }}"
         {{- if .Values.volumeMounts }}
         volumeMounts:
 {{ toYaml .Values.volumeMounts | indent 10 }}

deploy/charts/istio-csr/templates/role.yaml

@@ -26,4 +26,16 @@ rules:
   verbs: ["get", "list", "watch"]
   resourceNames: ["{{.Values.app.runtimeIssuanceConfigMap}}"]
 {{- end }}
-
+{{- if eq (toString .Values.app.tls.istiodCertificateEnable) "dynamic" }}
+- apiGroups:
+  - "cert-manager.io"
+  resources:
+  - "certificates"
+  verbs:
+  - "get"
+  - "create"
+  - "update"
+  - "delete"
+  - "watch"
+  - "list"
+{{- end }}

deploy/charts/istio-csr/values.schema.json

@@ -460,8 +460,7 @@
     },
     "helm-values.app.tls.istiodCertificateEnable": {
       "default": true,
-      "description": "Create the default certificate as part of install.",
-      "type": "boolean"
+      "description": "If true, create the istiod certificate using a cert-manager certificate as part of the install. If set to \"dynamic\", will create the cert dynamically when istio-csr pods start up. If false, no cert is created."
     },
     "helm-values.app.tls.istiodCertificateRenewBefore": {
       "default": "30m",

deploy/charts/istio-csr/values.yaml

@@ -134,7 +134,10 @@ app:
     # https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-204A.pdf
     certificateDuration: 1h
 
-    # Create the default certificate as part of install.
+    # If true, create the istiod certificate using a cert-manager certificate as part
+    # of the install. If set to "dynamic", will create the cert dynamically when
+    # istio-csr pods start up. If false, no cert is created.
+    # +docs:type=boolean,string,null
     istiodCertificateEnable: true
     # Requested duration of istio's Certificate. Will be automatically renewed.
     # Default is based on NIST 800-204A recommendations (SM-DR13).

make/config/istio-csr-pure-runtime-values.yaml

@@ -0,0 +1,45 @@
+replicaCount: 2
+
+service:
+  port: 443
+  type: NodePort
+  nodePort: 30443
+
+app:
+  logFormat: json
+  logLevel: 5
+
+  certmanager:
+    namespace: istio-system
+    # WARNING: do not enable this option in production, or environments with
+    # any non-trivial number of workloads for an extended period of time. Doing
+    # so will balloon the resource consumption of both ETCD and the API server,
+    # leading to errors and slow down. This option is intended for debugging
+    # purposes only, for limited periods of time.
+    preserveCertificateRequests: true
+    additionalAnnotations:
+    - name: custom.cert-manager.io/policy-name
+      value: istio-csr
+    issuer:
+      # Explicitly blanked out to test "pure" runtime configuration
+      group: ""
+      kind: ""
+      name: ""
+
+  tls:
+    trustDomain: foo.bar
+    istiodCertificateEnable: "dynamic"
+    certificateDuration: 20s
+    certificateDNSNames:
+    # Name used by the e2e client
+    - istio-csr.cert-manager.svc
+    # Name used within the demo cluster
+    - cert-manager-istio-csr.cert-manager.svc
+
+  server:
+    maxCertificateDuration: 5m
+    serving:
+      address: 0.0.0.0
+      port: 6443
+
+resources: {}

make/test-e2e.mk

@@ -48,11 +48,25 @@ e2e-create-cert-manager-istio-resources: | kind-cluster e2e-setup-cert-manager $
 	$(KUBECTL) create namespace istio-system || true
 	$(KUBECTL) -n istio-system apply --server-side -f ./make/config/cert-manager-bootstrap-resources.yaml
 
+.PHONY: e2e-create-cert-manager-istio-pure-runtime-resources
+e2e-create-cert-manager-istio-pure-runtime-resources: | kind-cluster e2e-setup-cert-manager $(NEEDS_KUBECTL)
+	$(KUBECTL) apply -f test/e2e-pure-runtime/initial-manifests/configmap.yaml
+
+is_e2e_test=
+
 # The "install" target can be run on its own with any currently active cluster,
 # we can't use any other cluster then a target containing "test-e2e" is run.
-# When a "test-e2e" target is run, the currently active cluster must be the kind
+# When a "test-e2e*" target is run, the currently active cluster must be the kind
 # cluster created by the "kind-cluster" target.
 ifeq ($(findstring test-e2e,$(MAKECMDGOALS)),test-e2e)
+is_e2e_test = yes
+endif
+
+ifeq ($(findstring test-e2e-pure-runtime,$(MAKECMDGOALS)),test-e2e-pure-runtime)
+is_e2e_test = yes
+endif
+
+ifdef is_e2e_test
 install: kind-cluster oci-load-manager e2e-create-cert-manager-istio-resources
 endif
 
@@ -100,3 +114,29 @@ test-e2e: test-e2e-deps | kind-cluster $(NEEDS_GINKGO) $(NEEDS_KUBECTL)
 		--kubeconfig-path $(CURDIR)/$(kind_kubeconfig) \
 		--kubectl-path $(KUBECTL) \
 		--runtime-issuance-config-map-name=$(E2E_RUNTIME_CONFIG_MAP_NAME)
+
+test-e2e-pure-runtime-deps: INSTALL_OPTIONS :=
+test-e2e-pure-runtime-deps: INSTALL_OPTIONS += --set image.repository=$(oci_manager_image_name_development)
+test-e2e-pure-runtime-deps: INSTALL_OPTIONS += --set app.runtimeIssuanceConfigMap=$(E2E_RUNTIME_CONFIG_MAP_NAME)
+test-e2e-pure-runtime-deps: INSTALL_OPTIONS += -f ./make/config/istio-csr-pure-runtime-values.yaml
+test-e2e-pure-runtime-deps: e2e-setup-cert-manager
+test-e2e-pure-runtime-deps: e2e-create-cert-manager-istio-resources
+test-e2e-pure-runtime-deps: e2e-create-cert-manager-istio-pure-runtime-resources
+test-e2e-pure-runtime-deps: install
+test-e2e-pure-runtime-deps: e2e-setup-istio
+
+# "Pure" runtime configuration e2e tests require different installation values
+.PHONY: test-e2e-pure-runtime
+test-e2e-pure-runtime: test-e2e-pure-runtime-deps | kind-cluster $(NEEDS_GINKGO) $(NEEDS_KUBECTL)
+	$(GINKGO) \
+		--output-dir=$(ARTIFACTS) \
+		--focus="$(E2E_FOCUS)" \
+		--junit-report=junit-go-e2e.xml \
+		$(EXTRA_GINKGO_FLAGS) \
+		./test/e2e-pure-runtime/ \
+		-ldflags $(go_manager_ldflags) \
+		-- \
+		--istioctl-path $(CURDIR)/$(bin_dir)/scratch/istioctl-$(ISTIO_VERSION) \
+		--kubeconfig-path $(CURDIR)/$(kind_kubeconfig) \
+		--kubectl-path $(KUBECTL) \
+		--runtime-issuance-config-map-name=$(E2E_RUNTIME_CONFIG_MAP_NAME)

pkg/certmanager/certmanager.go

@@ -75,6 +75,25 @@ type Signer interface {
 	Sign(ctx context.Context, identities string, csrPEM []byte, duration time.Duration, usages []cmapi.KeyUsage) (Bundle, error)
 }
 
+// IssuerChangeNotifier allows subscription to a channel providing updates on when an
+// issuer changes.
+type IssuerChangeNotifier interface {
+	// SubscribeIssuerChange provides a channel which will update with new issuerRefs
+	// as updates happen.
+	SubscribeIssuerChange() <-chan *cmmeta.ObjectReference
+
+	// MustWaitForIssuer returns true if there's no "default" issuerRef available
+	// (i.e. no static issuerRef was configured at startup)
+	// If this function returns true, InitialIssuer will always return nil and
+	// subscribers must wait for runtime configuration before trying to issue
+	// certificates
+	MustWaitForIssuer() bool
+
+	// InitialIssuer returns the "static" issuer which was configured at startup. Will
+	// always return nil if no such issuer exists.
+	InitialIssuer() *cmmeta.ObjectReference
+}
+
 // manager is used for signing CSRs via cert-manager. manager will create
 // CertificateRequests and wait for them to be signed, before returning the
 // result.
@@ -98,6 +117,9 @@ type manager struct {
 	// if no runtime configuration (ConfigMap configuration) is found, or if the
 	// ConfigMap for runtime configuration is deleted.
 	originalIssuerRef *cmmeta.ObjectReference
+
+	issuerChangeSubscriptions      []chan *cmmeta.ObjectReference
+	issuerChangeSubscriptionsMutex sync.Mutex
 }
 
 // Bundle represents the `status.Certificate` and `status.CA` that is
@@ -320,6 +342,8 @@ func (m *manager) handleRuntimeConfigIssuerChange(logger logr.Logger, event watc
 
 	logger.Info("Changed active issuerRef in response to runtime configuration ConfigMap", "issuer-name", m.activeIssuerRef.Name, "issuer-kind", m.activeIssuerRef.Kind, "issuer-group", m.activeIssuerRef.Group)
 
+	m.notifyIssuerChange(m.activeIssuerRef)
+
 	return nil
 }
 
@@ -336,6 +360,10 @@ func (m *manager) handleRuntimeConfigIssuerDeletion(logger logr.Logger) {
 	logger.Info("Runtime issuance configuration was deleted; issuance will revert to original issuerRef configured at install time")
 
 	m.activeIssuerRef = m.originalIssuerRef
+
+	// only send a nil pointer on the assumption that anything which cared about the original issuer ref
+	// kept track of it on startup
+	m.notifyIssuerChange(nil)
 }
 
 // RuntimeConfigurationWatcher is a wrapper around ctrlmgr.Runnable for watching runtime config
@@ -436,6 +464,35 @@ func (m *manager) RuntimeConfigurationWatcher(ctx context.Context) ctrlmgr.Runna
 	}
 }
 
+func (m *manager) SubscribeIssuerChange() <-chan *cmmeta.ObjectReference {
+	m.issuerChangeSubscriptionsMutex.Lock()
+	defer m.issuerChangeSubscriptionsMutex.Unlock()
+
+	ch := make(chan *cmmeta.ObjectReference)
+
+	m.issuerChangeSubscriptions = append(m.issuerChangeSubscriptions, ch)
+
+	return ch
+}
+
+func (m *manager) MustWaitForIssuer() bool {
+	// if no originalIssuerRef was configured, must wait for runtime configuration
+	return m.originalIssuerRef == nil
+}
+
+func (m *manager) InitialIssuer() *cmmeta.ObjectReference {
+	return m.originalIssuerRef
+}
+
+func (m *manager) notifyIssuerChange(issuerRef *cmmeta.ObjectReference) {
+	m.issuerChangeSubscriptionsMutex.Lock()
+	defer m.issuerChangeSubscriptionsMutex.Unlock()
+
+	for i := range m.issuerChangeSubscriptions {
+		go func(i int) { m.issuerChangeSubscriptions[i] <- issuerRef }(i)
+	}
+}
+
 var errNoOriginalIssuer = fmt.Errorf("no original issuer was provided")
 
 func handleOriginalIssuerRef(in cmmeta.ObjectReference) (*cmmeta.ObjectReference, error) {