Add a new issuer configuration to net-certmanager for system-internal…

…-tls certificates (#627)

* Add another issuerRef for system-internal-tls

* Update link to certmanager docs

* Review improvement

config/config.yaml

@@ -39,20 +39,29 @@ data:
     # These sample configuration options may be copied out of
     # this block and unindented to actually change the configuration.
 
-    # issuerRef is a reference to the issuer for cluster external certificates used for ingress.
+    # issuerRef is a reference to the issuer for external-domain certificates used for ingress.
     # IssuerRef should be either `ClusterIssuer` or `Issuer`.
-    # Please refer `IssuerRef` in https://github.com/cert-manager/cert-manager/tree/master/pkg/apis/certmanager/v1/types_certificate.go
+    # Please refer `IssuerRef` in https://cert-manager.io/docs/concepts/issuer/
     # for more details about IssuerRef configuration.
-    # If the issuerRef is not specified, the self-signed `knative-internal-encryption-ca` ClusterIssuer is used.
+    # If the issuerRef is not specified, the self-signed `knative-selfsigned-issuer` ClusterIssuer is used.
     issuerRef: |
       kind: ClusterIssuer
       name: letsencrypt-issuer
 
-    # clusterInternalIssuerRef is a reference to the issuer for cluster internal certificates used for ingress.
-    # ClusterInternalIssuerRef should be either `ClusterIssuer` or `Issuer`.
-    # Please refer `IssuerRef` in https://github.com/cert-manager/cert-manager/tree/master/pkg/apis/certmanager/v1/types_certificate.go
+    # clusterLocalIssuerRef is a reference to the issuer for cluster-local-domain certificates used for ingress.
+    # clusterLocalIssuerRef should be either `ClusterIssuer` or `Issuer`.
+    # Please refer `IssuerRef` in https://cert-manager.io/docs/concepts/issuer/
     # for more details about ClusterInternalIssuerRef configuration.
-    # If the clusterInternalIssuerRef is not specified, the self-signed `knative-internal-encryption-ca` ClusterIssuer is used.
-    clusterInternalIssuerRef: |
+    # If the clusterLocalIssuerRef is not specified, the self-signed `knative-selfsigned-issuer` ClusterIssuer is used.
+    clusterLocalIssuerRef: |
       kind: ClusterIssuer
-      name: knative-internal-encryption-issuer
+      name: your-company-issuer
+
+    # systemInternalIssuerRef is a reference to the issuer for certificates for system-internal-tls certificates used by Knative internal components.
+    # systemInternalIssuerRef should be either `ClusterIssuer` or `Issuer`.
+    # Please refer `IssuerRef` in https://cert-manager.io/docs/concepts/issuer/
+    # for more details about ClusterInternalIssuerRef configuration.
+    # If the systemInternalIssuerRef is not specified, the self-signed `knative-selfsigned-issuer` ClusterIssuer is used.
+    systemInternalIssuerRef: |
+      kind: ClusterIssuer
+      name: knative-selfsigned-issuer

config/knative-cluster-issuer.yaml

@@ -21,34 +21,37 @@ metadata:
     app.kubernetes.io/version: devel
     app.kubernetes.io/name: knative-serving
     networking.knative.dev/certificate-provider: cert-manager
+    knative.dev/issuer-install: "true"
 spec:
   selfSigned: {}
 ---
 apiVersion: cert-manager.io/v1
 kind: ClusterIssuer
 metadata:
-  name: knative-internal-encryption-issuer
+  name: knative-selfsigned-issuer
   labels:
     app.kubernetes.io/component: net-certmanager
     app.kubernetes.io/version: devel
     app.kubernetes.io/name: knative-serving
     networking.knative.dev/certificate-provider: cert-manager
+    knative.dev/issuer-install: "true"
 spec:
   ca:
-    secretName: knative-internal-encryption-ca
+    secretName: knative-selfsigned-ca
 ---
 apiVersion: cert-manager.io/v1
 kind: Certificate
 metadata:
-  name: knative-internal-encryption-ca
+  name: knative-selfsigned-ca
   namespace: cert-manager #  If you want to use it as a ClusterIssuer the secret must be in the cert-manager namespace.
   labels:
     app.kubernetes.io/component: net-certmanager
     app.kubernetes.io/version: devel
     app.kubernetes.io/name: knative-serving
     networking.knative.dev/certificate-provider: cert-manager
+    knative.dev/issuer-install: "true"
 spec:
-  secretName: knative-internal-encryption-ca
+  secretName: knative-selfsigned-ca
   commonName: knative.dev
   usages:
     - server auth

pkg/reconciler/certificate/certificate.go

@@ -52,7 +52,7 @@ import (
 
 const (
 	noCMConditionReason  = "NoCertManagerCertCondition"
-	noCMConditionMessage = "The ready condition of Cert Manager Certifiate does not exist."
+	noCMConditionMessage = "The ready condition of Cert Manager Certificate does not exist."
 	notReconciledReason  = "ReconcileFailed"
 	notReconciledMessage = "Cert-Manager certificate has not yet been reconciled."
 	httpDomainLabel      = "acme.cert-manager.io/http-domain"

pkg/reconciler/certificate/certificate_test.go

@@ -69,9 +69,15 @@ var (
 	notAfter          = &metav1.Time{
 		Time: time.Unix(123, 456),
 	}
-	clusterInternalIssuer = &cmv1.ClusterIssuer{
+	clusterLocalIssuer = &cmv1.ClusterIssuer{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "knative-internal-encryption-issuer",
+			Name: "knative-selfsigned-issuer",
+		},
+		Spec: cmv1.IssuerSpec{},
+	}
+	systemInternalIssuer = &cmv1.ClusterIssuer{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "knative-selfsigned-issuer",
 		},
 		Spec: cmv1.IssuerSpec{},
 	}
@@ -97,7 +103,8 @@ var (
 	}
 
 	externalCert, _                  = resources.MakeCertManagerCertificate(certmanagerConfig(), knCert("knCert", "foo"))
-	internalCert, _                  = resources.MakeCertManagerCertificate(certmanagerConfig(), withClusterLocalVisibility(knCert("knCert", "foo")))
+	localCert, _                     = resources.MakeCertManagerCertificate(certmanagerConfig(), withCertType(knCert("knCert", "foo"), netcfg.CertificateClusterLocalDomain))
+	systemInternalCert, _            = resources.MakeCertManagerCertificate(certmanagerConfig(), withCertType(knCert("knCert", "foo"), netcfg.CertificateSystemInternal))
 	externalCertShortenedDNSNames, _ = resources.MakeCertManagerCertificate(certmanagerConfig(), knCertShortenedDNSNames("knCert", "foo"))
 )
 
@@ -110,8 +117,7 @@ func TestNewController(t *testing.T) {
 			Namespace: system.Namespace(),
 		},
 		Data: map[string]string{
-			"issuerRef":                "kind: ClusterIssuer\nname: letsencrypt-issuer",
-			"clusterInternalIssuerRef": "kind: ClusterIssuer\nname: knative-internal-encryption-issuer",
+			"issuerRef": "kind: ClusterIssuer\nname: letsencrypt-issuer",
 		},
 	})
 
@@ -487,11 +493,11 @@ func TestReconcile(t *testing.T) {
 				}),
 		}},
 	}, {
-		Name: "create clusterInternalIssuer CM certificate matching Knative Certificate, with retry",
+		Name: "create clusterLocalIssuer CM certificate matching Knative Certificate, with retry",
 		Key:  "foo/knCert",
 		Objects: []runtime.Object{
-			withClusterLocalVisibility(knCert("knCert", "foo")),
-			clusterInternalIssuer,
+			withCertType(knCert("knCert", "foo"), netcfg.CertificateClusterLocalDomain),
+			clusterLocalIssuer,
 		},
 		WantErr: true,
 		WithReactors: []clientgotesting.ReactionFunc{
@@ -502,10 +508,10 @@ func TestReconcile(t *testing.T) {
 			Eventf(corev1.EventTypeWarning, "InternalError", "failed to create Cert-Manager Certificate: inducing failure for create certificates"),
 		},
 		WantCreates: []runtime.Object{
-			internalCert,
+			localCert,
 		},
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
-			Object: withClusterLocalVisibility(knCertWithStatus("knCert", "foo",
+			Object: withCertType(knCertWithStatus("knCert", "foo",
 				&v1alpha1.CertificateStatus{
 					Status: duckv1.Status{
 						ObservedGeneration: generation,
@@ -517,7 +523,40 @@ func TestReconcile(t *testing.T) {
 							Message:  notReconciledMessage,
 						}},
 					},
-				})),
+				}), netcfg.CertificateClusterLocalDomain),
+		}},
+	}, {
+		Name: "create systemInternalIssuer CM certificate matching Knative Certificate, with retry",
+		Key:  "foo/knCert",
+		Objects: []runtime.Object{
+			withCertType(knCert("knCert", "foo"), netcfg.CertificateSystemInternal),
+			systemInternalIssuer,
+		},
+		WantErr: true,
+		WithReactors: []clientgotesting.ReactionFunc{
+			InduceFailure("create", "certificates"),
+		},
+		WantEvents: []string{
+			Eventf(corev1.EventTypeWarning, "CreationFailed", "Failed to create Cert-Manager Certificate knCert/foo: inducing failure for create certificates"),
+			Eventf(corev1.EventTypeWarning, "InternalError", "failed to create Cert-Manager Certificate: inducing failure for create certificates"),
+		},
+		WantCreates: []runtime.Object{
+			systemInternalCert,
+		},
+		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
+			Object: withCertType(knCertWithStatus("knCert", "foo",
+				&v1alpha1.CertificateStatus{
+					Status: duckv1.Status{
+						ObservedGeneration: generation,
+						Conditions: duckv1.Conditions{{
+							Type:     v1alpha1.CertificateConditionReady,
+							Status:   corev1.ConditionUnknown,
+							Reason:   notReconciledReason,
+							Severity: apis.ConditionSeverityError,
+							Message:  notReconciledMessage,
+						}},
+					},
+				}), netcfg.CertificateSystemInternal),
 		}},
 	}}
 
@@ -747,9 +786,13 @@ func certmanagerConfig() *config.CertManagerConfig {
 			Kind: "ClusterIssuer",
 			Name: "Letsencrypt-issuer",
 		},
-		ClusterInternalIssuerRef: &cmmeta.ObjectReference{
+		ClusterLocalIssuerRef: &cmmeta.ObjectReference{
+			Kind: "ClusterIssuer",
+			Name: "knative-selfsigned-issuer",
+		},
+		SystemInternalIssuerRef: &cmmeta.ObjectReference{
 			Kind: "ClusterIssuer",
-			Name: "knative-internal-encryption-issuer",
+			Name: "knative-selfsigned-issuer",
 		},
 	}
 }
@@ -812,11 +855,11 @@ func knCertWithStatusAndGeneration(name, namespace string, status *v1alpha1.Cert
 	}
 }
 
-func withClusterLocalVisibility(certificate *v1alpha1.Certificate) *v1alpha1.Certificate {
+func withCertType(certificate *v1alpha1.Certificate, certType netcfg.CertificateType) *v1alpha1.Certificate {
 	if certificate.ObjectMeta.Labels == nil {
 		certificate.ObjectMeta.Labels = map[string]string{}
 	}
-	certificate.ObjectMeta.Labels[netapi.VisibilityLabelKey] = resources.VisibilityClusterLocal
+	certificate.ObjectMeta.Labels[netapi.CertificateTypeLabelKey] = string(certType)
 	return certificate
 }
 

pkg/reconciler/certificate/config/cert_manager.go

@@ -24,33 +24,36 @@ import (
 )
 
 const (
-	issuerRefKey                = "issuerRef"
-	clusterInternalIssuerRefKey = "clusterInternalIssuerRef"
+	issuerRefKey             = "issuerRef"
+	clusterLocalIssuerRefKey = "clusterLocalIssuerRef"
+	systemInternalIssuerRef  = "systemInternalIssuerRef"
 
 	// CertManagerConfigName is the name of the configmap containing all
 	// configuration related to Cert-Manager.
 	CertManagerConfigName = "config-certmanager"
 )
 
 // has to match the values in config/knative-cluster-issuer.yaml
-var knativeInternalIssuer = &cmeta.ObjectReference{
+var knativeSelfSignedIssuer = &cmeta.ObjectReference{
 	Kind: "ClusterIssuer",
-	Name: "knative-internal-encryption-ca",
+	Name: "knative-selfsigned-issuer",
 }
 
 // CertManagerConfig contains Cert-Manager related configuration defined in the
 // `config-certmanager` config map.
 type CertManagerConfig struct {
-	IssuerRef                *cmeta.ObjectReference
-	ClusterInternalIssuerRef *cmeta.ObjectReference
+	IssuerRef               *cmeta.ObjectReference
+	ClusterLocalIssuerRef   *cmeta.ObjectReference
+	SystemInternalIssuerRef *cmeta.ObjectReference
 }
 
 // NewCertManagerConfigFromConfigMap creates an CertManagerConfig from the supplied ConfigMap
 func NewCertManagerConfigFromConfigMap(configMap *corev1.ConfigMap) (*CertManagerConfig, error) {
 	// Use Knative self-signed ClusterIssuer as default
 	config := &CertManagerConfig{
-		IssuerRef:                knativeInternalIssuer,
-		ClusterInternalIssuerRef: knativeInternalIssuer,
+		IssuerRef:               knativeSelfSignedIssuer,
+		ClusterLocalIssuerRef:   knativeSelfSignedIssuer,
+		SystemInternalIssuerRef: knativeSelfSignedIssuer,
 	}
 
 	if v, ok := configMap.Data[issuerRefKey]; ok {
@@ -59,8 +62,14 @@ func NewCertManagerConfigFromConfigMap(configMap *corev1.ConfigMap) (*CertManage
 		}
 	}
 
-	if v, ok := configMap.Data[clusterInternalIssuerRefKey]; ok {
-		if err := yaml.Unmarshal([]byte(v), config.ClusterInternalIssuerRef); err != nil {
+	if v, ok := configMap.Data[clusterLocalIssuerRefKey]; ok {
+		if err := yaml.Unmarshal([]byte(v), config.ClusterLocalIssuerRef); err != nil {
+			return nil, err
+		}
+	}
+
+	if v, ok := configMap.Data[systemInternalIssuerRef]; ok {
+		if err := yaml.Unmarshal([]byte(v), config.SystemInternalIssuerRef); err != nil {
 			return nil, err
 		}
 	}

pkg/reconciler/certificate/config/cert_manager_test.go

@@ -56,8 +56,9 @@ func TestIssuerRef(t *testing.T) {
 				Name:      CertManagerConfigName,
 			},
 			Data: map[string]string{
-				issuerRefKey:                "wrong format",
-				clusterInternalIssuerRefKey: "wrong format",
+				issuerRefKey:             "wrong format",
+				clusterLocalIssuerRefKey: "wrong format",
+				systemInternalIssuerRef:  "wrong format",
 			},
 		},
 	}, {
@@ -68,7 +69,8 @@ func TestIssuerRef(t *testing.T) {
 				Name: "letsencrypt-issuer",
 				Kind: "ClusterIssuer",
 			},
-			ClusterInternalIssuerRef: knativeInternalIssuer,
+			ClusterLocalIssuerRef:   knativeSelfSignedIssuer,
+			SystemInternalIssuerRef: knativeSelfSignedIssuer,
 		},
 		config: &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
@@ -80,22 +82,43 @@ func TestIssuerRef(t *testing.T) {
 			},
 		},
 	}, {
-		name:    "valid clusterInternalIssuerRef",
+		name:    "valid ClusterLocalIssuerRef",
 		wantErr: false,
 		wantConfig: &CertManagerConfig{
-			IssuerRef: knativeInternalIssuer,
-			ClusterInternalIssuerRef: &cmmeta.ObjectReference{
-				Name: "knative-internal-encryption-issuer",
+			IssuerRef: knativeSelfSignedIssuer,
+			ClusterLocalIssuerRef: &cmmeta.ObjectReference{
+				Name: "cluster-local-issuer",
 				Kind: "ClusterIssuer",
 			},
+			SystemInternalIssuerRef: knativeSelfSignedIssuer,
 		},
 		config: &corev1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
 				Namespace: system.Namespace(),
 				Name:      CertManagerConfigName,
 			},
 			Data: map[string]string{
-				clusterInternalIssuerRefKey: "kind: ClusterIssuer\nname: knative-internal-encryption-issuer",
+				clusterLocalIssuerRefKey: "kind: ClusterIssuer\nname: cluster-local-issuer",
+			},
+		},
+	}, {
+		name:    "valid SystemInternalIssuerRef",
+		wantErr: false,
+		wantConfig: &CertManagerConfig{
+			IssuerRef:             knativeSelfSignedIssuer,
+			ClusterLocalIssuerRef: knativeSelfSignedIssuer,
+			SystemInternalIssuerRef: &cmmeta.ObjectReference{
+				Name: "system-internal-issuer",
+				Kind: "ClusterIssuer",
+			},
+		},
+		config: &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: system.Namespace(),
+				Name:      CertManagerConfigName,
+			},
+			Data: map[string]string{
+				clusterLocalIssuerRefKey: "kind: ClusterIssuer\nname: system-internal-issuer",
 			},
 		},
 	}}

pkg/reconciler/certificate/config/store_test.go

@@ -48,15 +48,22 @@ func TestStoreImmutableConfig(t *testing.T) {
 		Kind: "newKind",
 	}
 
-	config.CertManager.ClusterInternalIssuerRef = &cmeta.ObjectReference{
+	config.CertManager.ClusterLocalIssuerRef = &cmeta.ObjectReference{
+		Kind: "newKind",
+	}
+
+	config.CertManager.SystemInternalIssuerRef = &cmeta.ObjectReference{
 		Kind: "newKind",
 	}
 
 	newConfig := store.Load()
 	if newConfig.CertManager.IssuerRef != nil && newConfig.CertManager.IssuerRef.Kind == "newKind" {
 		t.Error("CertManager config is not immutable")
 	}
-	if newConfig.CertManager.ClusterInternalIssuerRef != nil && newConfig.CertManager.ClusterInternalIssuerRef.Kind == "newKind" {
+	if newConfig.CertManager.ClusterLocalIssuerRef != nil && newConfig.CertManager.ClusterLocalIssuerRef.Kind == "newKind" {
+		t.Error("CertManager config is not immutable")
+	}
+	if newConfig.CertManager.SystemInternalIssuerRef != nil && newConfig.CertManager.SystemInternalIssuerRef.Kind == "newKind" {
 		t.Error("CertManager config is not immutable")
 	}
 }