Cleanup SAN constants and Secrets Keys for system-internal-tls certif…

…icates (#861) * Cleanup knative-internal-tls constants and secrets * Drop `ServingControlCertName` constant
knative · Oct 2, 2023 · 463dc38 · 463dc38
1 parent 05d0964
commit 463dc38
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 111 deletions.
diff --git a/pkg/certificates/constants.go b/pkg/certificates/constants.go
@@ -20,41 +20,39 @@ import "strings"
 
 const (
 	Organization = "knative.dev"
-	//nolint:all
+
+	// nolint:all
 	LegacyFakeDnsName = "data-plane." + Organization
-	//nolint:all
-	// Deprecated: FakeDnsName is deprecated. Please use the DataPlaneRoutingName or DataPlaneUserName function.
-	FakeDnsName            = LegacyFakeDnsName
-	dataPlaneUserPrefix    = "kn-user-"
-	dataPlaneRoutingPrefix = "kn-routing-"
-	ControlPlaneName       = "kn-control"
-
-	//These keys are meant to line up with cert-manager, see
-	//https://cert-manager.io/docs/usage/certificate/#additional-certificate-output-formats
+
+	// nolint:all
+	// Deprecated: FakeDnsName is deprecated.
+	// Please use the DataPlaneRoutingSAN for calls to the Activator
+	// and the DataPlaneUserSAN function for calls to a Knative-Service via Queue-Proxy.
+	FakeDnsName = LegacyFakeDnsName
+
+	dataPlaneUserPrefix = "kn-user-"
+	DataPlaneRoutingSAN = "kn-routing"
+
+	// These keys are meant to line up with cert-manager, see
+	// https://cert-manager.io/docs/usage/certificate/#additional-certificate-output-formats
 	CaCertName     = "ca.crt"
 	CertName       = "tls.crt"
 	PrivateKeyName = "tls.key"
 
-	//These should be able to be deprecated some time in the future when the new names are fully adopted
+	// These should be able to be deprecated some time in the future when the new names are fully adopted
 	// #nosec
+	// Deprecated: please use CaCertName instead.
 	SecretCaCertKey = "ca-cert.pem"
 	// #nosec
+	// Deprecated: please use CertName instead.
 	SecretCertKey = "public-cert.pem"
 	// #nosec
+	// Deprecated: please use PrivateKeyName instead.
 	SecretPKKey = "private-key.pem"
 )
 
-// DataPlaneRoutingName constructs a san for a data-plane-routing certificate
-// Accepts a routingId  - a unique identifier used as part of the san (default is "0" used when an empty routingId is provided)
-func DataPlaneRoutingName(routingID string) string {
-	if routingID == "" {
-		routingID = "0"
-	}
-	return dataPlaneRoutingPrefix + strings.ToLower(routingID)
-}
-
-// DataPlaneUserName constructs a san for a data-plane-user certificate
-// Accepts a namespace  - the namespace for which the certificate was created
-func DataPlaneUserName(namespace string) string {
+// DataPlaneUserSAN constructs a SAN for a data-plane-user certificate in the
+// target namespace of a Knative Service.
+func DataPlaneUserSAN(namespace string) string {
 	return dataPlaneUserPrefix + strings.ToLower(namespace)
 }
diff --git a/pkg/certificates/reconciler/certificates.go b/pkg/certificates/reconciler/certificates.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package sample
+package reconciler
 
 import (
 	"bytes"
@@ -43,9 +43,6 @@ const (
 	expirationInterval   = time.Hour * 24 * 30       // 30 days
 	rotationThreshold    = 24 * time.Hour
 
-	// certificates used by control elements such as autoscaler, ingress controller
-	controlPlaneSecretType = "control-plane"
-
 	// certificates used by trusted data routing elements such as activator, ingress gw
 	dataPlaneRoutingSecretType = "data-plane-routing"
 
@@ -105,14 +102,10 @@ func (r *reconciler) ReconcileKind(ctx context.Context, secret *corev1.Secret) p
 	// Reconcile the provided secret
 	var sans []string
 	switch secret.Labels[r.secretTypeLabelName] {
-	case controlPlaneSecretType:
-		sans = []string{certificates.ControlPlaneName, certificates.LegacyFakeDnsName}
 	case dataPlaneRoutingSecretType:
-		routingID := secret.Labels[secretRoutingID]
-		san := certificates.DataPlaneRoutingName(routingID)
-		sans = []string{san, certificates.LegacyFakeDnsName}
+		sans = []string{certificates.DataPlaneRoutingSAN, certificates.LegacyFakeDnsName}
 	case dataPlaneUserSecretType:
-		sans = []string{certificates.DataPlaneUserName(secret.Namespace), certificates.LegacyFakeDnsName}
+		sans = []string{certificates.DataPlaneUserSAN(secret.Namespace), certificates.LegacyFakeDnsName}
 	case dataPlaneDeprecatedSecretType:
 		sans = []string{certificates.LegacyFakeDnsName}
 	default:

diff --git a/pkg/certificates/reconciler/certificates_test.go b/pkg/certificates/reconciler/certificates_test.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package sample
+package reconciler
 
 import (
 	"context"
@@ -83,26 +83,6 @@ func TestReconcile(t *testing.T) {
 		},
 	}
 
-	controlPlaneKP := mustCreateControlPlaneCert(t, expirationInterval, caKey, caCertificate)
-
-	wellFormedControlPlaneSecret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "control-plane-ctrl",
-			Namespace: namespace,
-			Labels: map[string]string{
-				labelName: controlPlaneSecretType,
-			},
-		},
-		Data: map[string][]byte{
-			certificates.SecretCaCertKey: caKP.CertBytes(),
-			certificates.SecretCertKey:   controlPlaneKP.CertBytes(),
-			certificates.SecretPKKey:     controlPlaneKP.PrivateKeyBytes(),
-			certificates.CaCertName:      caKP.CertBytes(),
-			certificates.CertName:        controlPlaneKP.CertBytes(),
-			certificates.PrivateKeyName:  controlPlaneKP.PrivateKeyBytes(),
-		},
-	}
-
 	dataPlaneUserKP := mustCreateDataPlaneUserCert(t, expirationInterval, caKey, caCertificate, "myns")
 
 	wellFormedDataPlaneUserSecret := &corev1.Secret{
@@ -123,7 +103,7 @@ func TestReconcile(t *testing.T) {
 		},
 	}
 
-	dataPlaneRoutingKP := mustCreateDataPlaneRoutingCert(t, 10*time.Hour, caKey, caCertificate, "0")
+	dataPlaneRoutingKP := mustCreateDataPlaneRoutingCert(t, 10*time.Hour, caKey, caCertificate)
 
 	wellFormedDataPlaneRoutingSecret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -150,16 +130,13 @@ func TestReconcile(t *testing.T) {
 		objects                []*corev1.Secret
 		asserts                map[string]func(*testing.T, *corev1.Secret)
 	}{{
-		name:    "well formed secret CA and control plane secret exists",
-		key:     namespace + "/control-plane-ctrl",
-		objects: []*corev1.Secret{wellFormedCaSecret, wellFormedControlPlaneSecret},
+		name:    "well formed secret CA exists",
+		key:     namespace + "/data-plane-ctrl",
+		objects: []*corev1.Secret{wellFormedCaSecret},
 		asserts: map[string]func(*testing.T, *corev1.Secret){
 			wellFormedCaSecret.Name: func(t *testing.T, secret *corev1.Secret) {
 				require.Equal(t, wellFormedCaSecret, secret)
 			},
-			wellFormedControlPlaneSecret.Name: func(t *testing.T, secret *corev1.Secret) {
-				require.Equal(t, wellFormedControlPlaneSecret, secret)
-			},
 		},
 	}, {
 		name:    "well formed secret CA and data plane user secret exists",
@@ -186,8 +163,8 @@ func TestReconcile(t *testing.T) {
 			},
 		},
 	}, {
-		name:                   "empty CA secret and empty control plane secret",
-		key:                    namespace + "/control-plane-ctrl",
+		name:                   "empty CA secret and empty data plane secret",
+		key:                    namespace + "/data-plane-ctrl",
 		executeReconcilerTwice: true,
 		objects: []*corev1.Secret{{
 			ObjectMeta: metav1.ObjectMeta{
@@ -196,16 +173,16 @@ func TestReconcile(t *testing.T) {
 			},
 		}, {
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "control-plane-ctrl",
+				Name:      "data-plane-ctrl",
 				Namespace: namespace,
 				Labels: map[string]string{
-					labelName: controlPlaneSecretType,
+					labelName: dataPlaneRoutingSecretType,
 				},
 			},
 		}},
 		asserts: map[string]func(*testing.T, *corev1.Secret){
-			caSecretName:         validCACert,
-			"control-plane-ctrl": validControlPlaneCert,
+			caSecretName:      validCACert,
+			"data-plane-ctrl": validDataPlaneCert,
 		},
 	}, {
 		name:                   "empty CA secret and empty data plane user secret",
@@ -227,7 +204,7 @@ func TestReconcile(t *testing.T) {
 		}},
 		asserts: map[string]func(*testing.T, *corev1.Secret){
 			caSecretName:      validCACert,
-			"data-plane-ctrl": validControlPlaneCert,
+			"data-plane-ctrl": validDataPlaneCert,
 		},
 	}, {
 		name:                   "empty CA secret and empty data plane routing secret",
@@ -249,29 +226,29 @@ func TestReconcile(t *testing.T) {
 		}},
 		asserts: map[string]func(*testing.T, *corev1.Secret){
 			caSecretName:      validCACert,
-			"data-plane-ctrl": validControlPlaneCert,
+			"data-plane-ctrl": validDataPlaneCert,
 		},
 	}, {
-		name: "well formed secret CA but empty control plane secret",
-		key:  namespace + "/control-plane-ctrl",
+		name: "well formed secret CA but empty data plane secret",
+		key:  namespace + "/data-plane-ctrl",
 		objects: []*corev1.Secret{wellFormedCaSecret, {
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "control-plane-ctrl",
+				Name:      "data-plane-ctrl",
 				Namespace: namespace,
 				Labels: map[string]string{
-					labelName: controlPlaneSecretType,
+					labelName: dataPlaneRoutingSecretType,
 				},
 			},
 		}},
 		asserts: map[string]func(*testing.T, *corev1.Secret){
 			wellFormedCaSecret.Name: func(t *testing.T, secret *corev1.Secret) {
 				require.Equal(t, wellFormedCaSecret, secret)
 			},
-			"control-plane-ctrl": validControlPlaneCert,
+			"data-plane-ctrl": validDataPlaneCert,
 		},
 	}, {
-		name:                   "malformed secret CA and malformed control plane secret",
-		key:                    namespace + "/control-plane-ctrl",
+		name:                   "malformed secret CA and malformed data plane secret",
+		key:                    namespace + "/data-plane-ctrl",
 		executeReconcilerTwice: true,
 		objects: []*corev1.Secret{{
 			ObjectMeta: metav1.ObjectMeta{
@@ -283,10 +260,10 @@ func TestReconcile(t *testing.T) {
 			},
 		}, {
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "control-plane-ctrl",
+				Name:      "data-plane-ctrl",
 				Namespace: namespace,
 				Labels: map[string]string{
-					labelName: controlPlaneSecretType,
+					labelName: dataPlaneRoutingSecretType,
 				},
 			},
 			Data: map[string][]byte{
@@ -296,34 +273,34 @@ func TestReconcile(t *testing.T) {
 			},
 		}},
 		asserts: map[string]func(*testing.T, *corev1.Secret){
-			caSecretName:         validCACert,
-			"control-plane-ctrl": validControlPlaneCert,
+			caSecretName:      validCACert,
+			"data-plane-ctrl": validDataPlaneCert,
 		},
 	}, {
-		name: "well formed secret CA and malformed control plane secret",
-		key:  namespace + "/control-plane-ctrl",
+		name: "well formed secret CA and malformed data plane secret",
+		key:  namespace + "/data-plane-ctrl",
 		objects: []*corev1.Secret{wellFormedCaSecret, {
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "control-plane-ctrl",
+				Name:      "data-plane-ctrl",
 				Namespace: namespace,
 				Labels: map[string]string{
-					labelName: controlPlaneSecretType,
+					labelName: dataPlaneRoutingSecretType,
 				},
 			},
 		}},
 		asserts: map[string]func(*testing.T, *corev1.Secret){
-			caSecretName:         validCACert,
-			"control-plane-ctrl": validControlPlaneCert,
+			caSecretName:      validCACert,
+			"data-plane-ctrl": validDataPlaneCert,
 		},
 	}, {
-		name: "no CA secret and empty control plane secret",
-		key:  namespace + "/control-plane-ctrl",
+		name: "no CA secret and empty data plane secret",
+		key:  namespace + "/data-plane-ctrl",
 		objects: []*corev1.Secret{{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "control-plane-ctrl",
+				Name:      "data-plane-ctrl",
 				Namespace: namespace,
 				Labels: map[string]string{
-					labelName: controlPlaneSecretType,
+					labelName: dataPlaneRoutingSecretType,
 				},
 			},
 		}},
@@ -369,19 +346,13 @@ func mustCreateCACert(t *testing.T, expirationInterval time.Duration) (*certific
 }
 
 func mustCreateDataPlaneUserCert(t *testing.T, expirationInterval time.Duration, caKey *rsa.PrivateKey, caCertificate *x509.Certificate, namespace string) *certificates.KeyPair {
-	kp, err := certificates.CreateCert(caKey, caCertificate, expirationInterval, certificates.DataPlaneUserName(namespace), certificates.LegacyFakeDnsName)
-	require.NoError(t, err)
-	return kp
-}
-
-func mustCreateDataPlaneRoutingCert(t *testing.T, expirationInterval time.Duration, caKey *rsa.PrivateKey, caCertificate *x509.Certificate, routingID string) *certificates.KeyPair {
-	kp, err := certificates.CreateCert(caKey, caCertificate, expirationInterval, certificates.DataPlaneRoutingName(routingID), certificates.LegacyFakeDnsName)
+	kp, err := certificates.CreateCert(caKey, caCertificate, expirationInterval, certificates.DataPlaneUserSAN(namespace), certificates.LegacyFakeDnsName)
 	require.NoError(t, err)
 	return kp
 }
 
-func mustCreateControlPlaneCert(t *testing.T, expirationInterval time.Duration, caKey *rsa.PrivateKey, caCertificate *x509.Certificate) *certificates.KeyPair {
-	kp, err := certificates.CreateCert(caKey, caCertificate, expirationInterval, certificates.ControlPlaneName, certificates.LegacyFakeDnsName)
+func mustCreateDataPlaneRoutingCert(t *testing.T, expirationInterval time.Duration, caKey *rsa.PrivateKey, caCertificate *x509.Certificate) *certificates.KeyPair {
+	kp, err := certificates.CreateCert(caKey, caCertificate, expirationInterval, certificates.DataPlaneRoutingSAN, certificates.LegacyFakeDnsName)
 	require.NoError(t, err)
 	return kp
 }
@@ -409,5 +380,3 @@ func validDataPlaneCert(t *testing.T, secret *corev1.Secret) {
 
 	validCACert(t, secret)
 }
-
-var validControlPlaneCert = validDataPlaneCert
diff --git a/pkg/certificates/reconciler/controller.go b/pkg/certificates/reconciler/controller.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package sample
+package reconciler
 
 import (
 	"context"
@@ -38,7 +38,6 @@ import (
 const (
 	caSecretNamePostfix    = "-ctrl-ca"
 	secretLabelNamePostfix = "-ctrl"
-	secretRoutingID        = "routing-id"
 )
 
 // NewControllerFactory generates a ControllerConstructor for the control certificates reconciler.

diff --git a/pkg/certificates/reconciler/controller_impl.go b/pkg/certificates/reconciler/controller_impl.go
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package sample
+package reconciler
 
 import (
 	context "context"

diff --git a/pkg/certificates/reconciler/reconciler.go b/pkg/certificates/reconciler/reconciler.go
diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -70,17 +70,12 @@ const (
 	// ServingInternalCertName is the name of secret contains certificates in serving
 	// system namespace.
 	//
-	// Deprecated: ServingInternalCertName is deprecated.
-	// (use ServingControlCertName or ServingRoutingCertName instead)
+	// Deprecated: ServingInternalCertName is deprecated. Use ServingRoutingCertName instead.
 	ServingInternalCertName = "knative-serving-certs"
 
 	// ServingRoutingCertName is the name of secret contains certificates for Routing data in serving
 	// system namespace. (Used by Ingress GWs and Activator)
 	ServingRoutingCertName = "routing-serving-certs"
-
-	// ServingControlCertName is the name of secret contains certificates for Control data in serving
-	// system namespace. (Used by Autoscaler and Ingress control for example)
-	ServingControlCertName = "control-serving-certs"
 )
 
 // Config Keys