From c2f8ce131ee7f84334f4bc4e63fad7fa4a6d8261 Mon Sep 17 00:00:00 2001 From: Jim Crossley Date: Wed, 13 Nov 2019 17:12:19 -0500 Subject: [PATCH 1/4] Add CustomCerts to enable controller to trust self-signed registry Modified CRD and ran update-codegen.sh --- ...300-serving-v1alpha1-knativeserving-crd.yaml | 11 +++++++++++ .../serving/v1alpha1/knativeserving_types.go | 13 +++++++++++++ .../serving/v1alpha1/zz_generated.deepcopy.go | 17 +++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/config/300-serving-v1alpha1-knativeserving-crd.yaml b/config/300-serving-v1alpha1-knativeserving-crd.yaml index 18cf72ca..ddf8dd3d 100644 --- a/config/300-serving-v1alpha1-knativeserving-crd.yaml +++ b/config/300-serving-v1alpha1-knativeserving-crd.yaml @@ -93,6 +93,17 @@ spec: name: description: The name of the secret. type: string + controller-custom-certs: + description: Enabling the controller to trust registries with self-signed certificates + type: object + properties: + type: + description: One of ConfigMap or Secret + type: string + enum: [ConfigMap, Secret] + name: + description: The name of the ConfigMap or Secret + type: string type: object status: description: Status defines the observed state of KnativeServing diff --git a/pkg/apis/serving/v1alpha1/knativeserving_types.go b/pkg/apis/serving/v1alpha1/knativeserving_types.go index 895c97ce..ac0fe006 100644 --- a/pkg/apis/serving/v1alpha1/knativeserving_types.go +++ b/pkg/apis/serving/v1alpha1/knativeserving_types.go @@ -57,6 +57,16 @@ type IstioGatewayOverride struct { Selector map[string]string `json:"selector,omitempty"` } +// Refers to either a ConfigMap or Secret containing valid CA +// certificates +type CustomCerts struct { + // One of ConfigMap or Secret + Type string `json:"type"` + + // The name of the ConfigMap or Secret + Name string `json:"name"` +} + // KnativeServingSpec defines the desired state of KnativeServing // +k8s:openapi-gen=true type KnativeServingSpec struct { @@ -78,6 +88,9 @@ type KnativeServingSpec struct { // A means to override the cluster-local-gateway ClusterLocalGateway IstioGatewayOverride `json:"cluster-local-gateway,omitempty"` + + // Enables controller to trust registries with self-signed certificates + ControllerCustomCerts CustomCerts `json:"controller-custom-certs,omitempty"` } // KnativeServingStatus defines the observed state of KnativeServing diff --git a/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go index 126dcb39..0cff0237 100644 --- a/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go @@ -26,6 +26,22 @@ import ( apis "knative.dev/pkg/apis" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CustomCerts) DeepCopyInto(out *CustomCerts) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CustomCerts. +func (in *CustomCerts) DeepCopy() *CustomCerts { + if in == nil { + return nil + } + out := new(CustomCerts) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IstioGatewayOverride) DeepCopyInto(out *IstioGatewayOverride) { *out = *in @@ -133,6 +149,7 @@ func (in *KnativeServingSpec) DeepCopyInto(out *KnativeServingSpec) { in.Registry.DeepCopyInto(&out.Registry) in.KnativeIngressGateway.DeepCopyInto(&out.KnativeIngressGateway) in.ClusterLocalGateway.DeepCopyInto(&out.ClusterLocalGateway) + out.ControllerCustomCerts = in.ControllerCustomCerts return } From 9daa2fb47fe5beafd9716e8b96ae7a0ab928d47f Mon Sep 17 00:00:00 2001 From: Jim Crossley Date: Wed, 13 Nov 2019 17:14:24 -0500 Subject: [PATCH 2/4] Add a CustomCerts Transformer with unit tests An e2e test will require a well-known self-signed registry, but I did verify it worked manually --- pkg/reconciler/knativeserving/common/certs.go | 81 +++++++++ .../knativeserving/common/certs_test.go | 158 ++++++++++++++++++ .../knativeserving/common/extensions.go | 1 + 3 files changed, 240 insertions(+) create mode 100644 pkg/reconciler/knativeserving/common/certs.go create mode 100644 pkg/reconciler/knativeserving/common/certs_test.go diff --git a/pkg/reconciler/knativeserving/common/certs.go b/pkg/reconciler/knativeserving/common/certs.go new file mode 100644 index 00000000..220bc254 --- /dev/null +++ b/pkg/reconciler/knativeserving/common/certs.go @@ -0,0 +1,81 @@ +package common + +import ( + "errors" + "fmt" + + mf "github.com/jcrossley3/manifestival" + "go.uber.org/zap" + appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/kubernetes/scheme" + servingv1alpha1 "knative.dev/serving-operator/pkg/apis/serving/v1alpha1" +) + +const ( + customCertsEnvName = "SSL_CERT_DIR" + customCertsMountPath = "/custom-certs" + customCertsNamePrefix = "custom-certs-" +) + +func CustomCertsTransform(instance *servingv1alpha1.KnativeServing, log *zap.SugaredLogger) mf.Transformer { + empty := servingv1alpha1.CustomCerts{} + return func(u *unstructured.Unstructured) error { + if instance.Spec.ControllerCustomCerts == empty { + return nil + } + if u.GetKind() == "Deployment" && u.GetName() == "controller" { + certs := instance.Spec.ControllerCustomCerts + deployment := &appsv1.Deployment{} + if err := scheme.Scheme.Convert(u, deployment, nil); err != nil { + return err + } + if err := configureCustomCerts(deployment, certs); err != nil { + return err + } + if err := scheme.Scheme.Convert(deployment, u, nil); err != nil { + return err + } + } + return nil + } +} + +func configureCustomCerts(deployment *appsv1.Deployment, certs servingv1alpha1.CustomCerts) error { + source := v1.VolumeSource{} + switch certs.Type { + case "ConfigMap": + source.ConfigMap = &v1.ConfigMapVolumeSource{ + LocalObjectReference: v1.LocalObjectReference{ + Name: certs.Name, + }, + } + case "Secret": + source.Secret = &v1.SecretVolumeSource{ + SecretName: certs.Name, + } + default: + return errors.New(fmt.Sprintf("Unknown CustomCerts type: %s", certs.Type)) + } + + name := customCertsNamePrefix + certs.Name + if name == customCertsNamePrefix { + return errors.New(fmt.Sprintf("CustomCerts name for %s is required", certs.Type)) + } + deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, v1.Volume{ + Name: name, + VolumeSource: source, + }) + + containers := deployment.Spec.Template.Spec.Containers + containers[0].VolumeMounts = append(containers[0].VolumeMounts, v1.VolumeMount{ + Name: name, + MountPath: customCertsMountPath, + }) + containers[0].Env = append(containers[0].Env, v1.EnvVar{ + Name: customCertsEnvName, + Value: customCertsMountPath, + }) + return nil +} diff --git a/pkg/reconciler/knativeserving/common/certs_test.go b/pkg/reconciler/knativeserving/common/certs_test.go new file mode 100644 index 00000000..3214d7ec --- /dev/null +++ b/pkg/reconciler/knativeserving/common/certs_test.go @@ -0,0 +1,158 @@ +/* +Copyright 2019 The Knative 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 common + +import ( + "testing" + + servingv1alpha1 "knative.dev/serving-operator/pkg/apis/serving/v1alpha1" + + appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/kubernetes/scheme" +) + +type customCertsTest struct { + name string + input servingv1alpha1.CustomCerts + expectError bool + expectSource *v1.VolumeSource +} + +var customCertsTests = []customCertsTest{ + { + name: "FromSecret", + input: servingv1alpha1.CustomCerts{ + Type: "Secret", + Name: "my-secret", + }, + expectError: false, + expectSource: &v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: "my-secret", + }, + }, + }, + { + name: "FromConfigMap", + input: servingv1alpha1.CustomCerts{ + Type: "ConfigMap", + Name: "my-map", + }, + expectError: false, + expectSource: &v1.VolumeSource{ + ConfigMap: &v1.ConfigMapVolumeSource{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "my-map", + }, + }, + }, + }, + { + name: "NoCerts", + input: servingv1alpha1.CustomCerts{}, + expectError: false, + }, + { + name: "InvalidType", + input: servingv1alpha1.CustomCerts{ + Type: "invalid", + }, + expectError: true, + }, + { + name: "MissingName", + input: servingv1alpha1.CustomCerts{ + Type: "Secret", + }, + expectError: true, + }, +} + +func TestOnlyTransformCustomCertsForController(t *testing.T) { + before := makeDeployment(t, "not-controller", v1.PodSpec{ + Containers: []v1.Container{{ + Name: "definitely-not-controller", + }}, + }) + instance := &servingv1alpha1.KnativeServing{ + Spec: servingv1alpha1.KnativeServingSpec{ + ControllerCustomCerts: servingv1alpha1.CustomCerts{ + Type: "Secret", + Name: "my-secret", + }, + }, + } + customCertsTransform := CustomCertsTransform(instance, log) + unstructured := makeUnstructured(t, before) + err := customCertsTransform(&unstructured) + assertEqual(t, err, nil) + after := &appsv1.Deployment{} + err = scheme.Scheme.Convert(&unstructured, after, nil) + assertEqual(t, err, nil) + assertDeepEqual(t, after.Spec, before.Spec) +} + +func TestCustomCertsTransform(t *testing.T) { + for _, tt := range customCertsTests { + t.Run(tt.name, func(t *testing.T) { + runCustomCertsTransformTest(t, &tt) + }) + } +} + +func runCustomCertsTransformTest(t *testing.T, tt *customCertsTest) { + unstructured := makeUnstructured(t, makeDeployment(t, "controller", v1.PodSpec{ + Containers: []v1.Container{{ + Name: "controller", + }}, + })) + instance := &servingv1alpha1.KnativeServing{ + Spec: servingv1alpha1.KnativeServingSpec{ + ControllerCustomCerts: tt.input, + }, + } + customCertsTransform := CustomCertsTransform(instance, log) + err := customCertsTransform(&unstructured) + if tt.expectError && err == nil { + t.Fatal("Transformer should've returned an error and did not") + } + validateCustomCertsTransform(t, tt, &unstructured) +} + +func validateCustomCertsTransform(t *testing.T, tt *customCertsTest, u *unstructured.Unstructured) { + deployment := &appsv1.Deployment{} + err := scheme.Scheme.Convert(u, deployment, nil) + assertEqual(t, err, nil) + spec := deployment.Spec.Template.Spec + if tt.expectSource != nil { + assertEqual(t, spec.Volumes[0].Name, customCertsNamePrefix+tt.input.Name) + assertDeepEqual(t, &spec.Volumes[0].VolumeSource, tt.expectSource) + assertDeepEqual(t, spec.Containers[0].Env[0], v1.EnvVar{ + Name: customCertsEnvName, + Value: customCertsMountPath, + }) + assertDeepEqual(t, spec.Containers[0].VolumeMounts[0], v1.VolumeMount{ + Name: customCertsNamePrefix + tt.input.Name, + MountPath: customCertsMountPath, + }) + } else { + assertEqual(t, len(spec.Volumes), 0) + assertEqual(t, len(spec.Containers[0].Env), 0) + assertEqual(t, len(spec.Containers[0].VolumeMounts), 0) + } +} diff --git a/pkg/reconciler/knativeserving/common/extensions.go b/pkg/reconciler/knativeserving/common/extensions.go index 17151f55..50bac1fa 100644 --- a/pkg/reconciler/knativeserving/common/extensions.go +++ b/pkg/reconciler/knativeserving/common/extensions.go @@ -35,6 +35,7 @@ func (platforms Platforms) Transformers(kubeClientSet kubernetes.Interface, inst DeploymentTransform(instance, log), ImageTransform(instance, log), GatewayTransform(instance, log), + CustomCertsTransform(instance, log), } for _, fn := range platforms { transformer, err := fn(kubeClientSet, log) From 7ec600442627dcbb46fd83f87687d7a0fd4da959 Mon Sep 17 00:00:00 2001 From: Jim Crossley Date: Wed, 13 Nov 2019 17:43:18 -0500 Subject: [PATCH 3/4] Fixed prow bot linter errors --- .../serving/v1alpha1/knativeserving_types.go | 4 ++-- pkg/reconciler/knativeserving/common/certs.go | 23 ++++++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/pkg/apis/serving/v1alpha1/knativeserving_types.go b/pkg/apis/serving/v1alpha1/knativeserving_types.go index ac0fe006..c4ea7320 100644 --- a/pkg/apis/serving/v1alpha1/knativeserving_types.go +++ b/pkg/apis/serving/v1alpha1/knativeserving_types.go @@ -57,8 +57,8 @@ type IstioGatewayOverride struct { Selector map[string]string `json:"selector,omitempty"` } -// Refers to either a ConfigMap or Secret containing valid CA -// certificates +// CustomCerts refers to either a ConfigMap or Secret containing valid +// CA certificates type CustomCerts struct { // One of ConfigMap or Secret Type string `json:"type"` diff --git a/pkg/reconciler/knativeserving/common/certs.go b/pkg/reconciler/knativeserving/common/certs.go index 220bc254..26bdfcc5 100644 --- a/pkg/reconciler/knativeserving/common/certs.go +++ b/pkg/reconciler/knativeserving/common/certs.go @@ -1,7 +1,22 @@ +/* +Copyright 2019 The Knative 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 common import ( - "errors" "fmt" mf "github.com/jcrossley3/manifestival" @@ -19,6 +34,8 @@ const ( customCertsNamePrefix = "custom-certs-" ) +// CustomCertsTransform configures the controller deployment to trust +// registries with self-signed certs func CustomCertsTransform(instance *servingv1alpha1.KnativeServing, log *zap.SugaredLogger) mf.Transformer { empty := servingv1alpha1.CustomCerts{} return func(u *unstructured.Unstructured) error { @@ -56,12 +73,12 @@ func configureCustomCerts(deployment *appsv1.Deployment, certs servingv1alpha1.C SecretName: certs.Name, } default: - return errors.New(fmt.Sprintf("Unknown CustomCerts type: %s", certs.Type)) + return fmt.Errorf("Unknown CustomCerts type: %s", certs.Type) } name := customCertsNamePrefix + certs.Name if name == customCertsNamePrefix { - return errors.New(fmt.Sprintf("CustomCerts name for %s is required", certs.Type)) + return fmt.Errorf("CustomCerts name for %s is required", certs.Type) } deployment.Spec.Template.Spec.Volumes = append(deployment.Spec.Template.Spec.Volumes, v1.Volume{ Name: name, From b32ce7a775fb7c5fd5978e11c967714bc863fcfc Mon Sep 17 00:00:00 2001 From: Jim Crossley Date: Wed, 13 Nov 2019 18:01:33 -0500 Subject: [PATCH 4/4] Added empty string to CRD enum to fix e2e test This seems wrong, but maybe not quite as wrong as the client sending empty structs for all the optional fields in the CR. --- config/300-serving-v1alpha1-knativeserving-crd.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/config/300-serving-v1alpha1-knativeserving-crd.yaml b/config/300-serving-v1alpha1-knativeserving-crd.yaml index ddf8dd3d..3828315e 100644 --- a/config/300-serving-v1alpha1-knativeserving-crd.yaml +++ b/config/300-serving-v1alpha1-knativeserving-crd.yaml @@ -100,7 +100,10 @@ spec: type: description: One of ConfigMap or Secret type: string - enum: [ConfigMap, Secret] + enum: + - ConfigMap + - Secret + - "" name: description: The name of the ConfigMap or Secret type: string