diff --git a/api/v1beta2/provider_types.go b/api/v1beta2/provider_types.go index 178f733d4..b5fd94835 100644 --- a/api/v1beta2/provider_types.go +++ b/api/v1beta2/provider_types.go @@ -103,8 +103,11 @@ type ProviderSpec struct { SecretRef *meta.LocalObjectReference `json:"secretRef,omitempty"` // CertSecretRef specifies the Secret containing - // a PEM-encoded CA certificate (`caFile`). + // a PEM-encoded CA certificate (in the `ca.crt` key). // +optional + // + // Note: Support for the `caFile` key has + // been deprecated. CertSecretRef *meta.LocalObjectReference `json:"certSecretRef,omitempty"` // Suspend tells the controller to suspend subsequent diff --git a/config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml b/config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml index a8b7ce2f1..88f1c99e6 100644 --- a/config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml +++ b/config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml @@ -236,8 +236,9 @@ spec: maxLength: 2048 type: string certSecretRef: - description: CertSecretRef specifies the Secret containing a PEM-encoded - CA certificate (`caFile`). + description: "CertSecretRef specifies the Secret containing a PEM-encoded + CA certificate (in the `ca.crt` key). \n Note: Support for the `caFile` + key has been deprecated." properties: name: description: Name of the referent. diff --git a/docs/api/v1beta2/notification.md b/docs/api/v1beta2/notification.md index a47b9e719..59c5de580 100644 --- a/docs/api/v1beta2/notification.md +++ b/docs/api/v1beta2/notification.md @@ -372,7 +372,9 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference (Optional)

CertSecretRef specifies the Secret containing -a PEM-encoded CA certificate (caFile).

+a PEM-encoded CA certificate (in the ca.crt key).

+

Note: Support for the caFile key has +been deprecated.

@@ -964,7 +966,9 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference (Optional)

CertSecretRef specifies the Secret containing -a PEM-encoded CA certificate (caFile).

+a PEM-encoded CA certificate (in the ca.crt key).

+

Note: Support for the caFile key has +been deprecated.

diff --git a/docs/spec/v1beta2/providers.md b/docs/spec/v1beta2/providers.md index 1f60fd213..644ce34e5 100644 --- a/docs/spec/v1beta2/providers.md +++ b/docs/spec/v1beta2/providers.md @@ -1095,11 +1095,12 @@ stringData: `.spec.certSecretRef` is an optional field to specify a name reference to a Secret in the same namespace as the Provider, containing the TLS CA certificate. +The secret must be of type `kubernetes.io/tls` or `Opaque`. #### Example To enable notification-controller to communicate with a provider API over HTTPS -using a self-signed TLS certificate, set the `caFile` like so: +using a self-signed TLS certificate, set the `ca.crt` like so: ```yaml --- @@ -1119,11 +1120,16 @@ kind: Secret metadata: name: my-ca-crt namespace: default +type: kubernetes.io/tls # or Opaque stringData: - caFile: | + ca.crt: | <--- CA Key ---> ``` +**Warning:** Support for the `caFile` key has been +deprecated. If you have any Secrets using this key, +the controller will log a deprecation warning. + ### HTTP/S proxy `.spec.proxy` is an optional field to specify an HTTP/S proxy address. diff --git a/internal/controller/alert_controller.go b/internal/controller/alert_controller.go index d22e52e6d..0616f232a 100644 --- a/internal/controller/alert_controller.go +++ b/internal/controller/alert_controller.go @@ -178,7 +178,7 @@ func (r *AlertReconciler) isProviderReady(ctx context.Context, alert *apiv1beta2 providerName := types.NamespacedName{Namespace: alert.Namespace, Name: alert.Spec.ProviderRef.Name} if err := r.Get(ctx, providerName, provider); err != nil { // log not found errors since they get filtered out - ctrl.LoggerFrom(ctx).Error(err, "failed to get provider %s", providerName.String()) + ctrl.LoggerFrom(ctx).Error(err, "failed to get provider", "provider", providerName.String()) return fmt.Errorf("failed to get provider '%s': %w", providerName.String(), err) } diff --git a/internal/controller/provider_controller.go b/internal/controller/provider_controller.go index 8b07b052d..d1b753b44 100644 --- a/internal/controller/provider_controller.go +++ b/internal/controller/provider_controller.go @@ -190,6 +190,8 @@ func (r *ProviderReconciler) validateURLs(provider *apiv1beta2.Provider) error { } func (r *ProviderReconciler) validateCredentials(ctx context.Context, provider *apiv1beta2.Provider) error { + log := ctrl.LoggerFrom(ctx) + address := provider.Spec.Address proxy := provider.Spec.Proxy username := provider.Spec.Username @@ -245,9 +247,19 @@ func (r *ProviderReconciler) validateCredentials(ctx context.Context, provider * return fmt.Errorf("failed to read secret, error: %w", err) } - caFile, ok := secret.Data["caFile"] + switch secret.Type { + case corev1.SecretTypeOpaque, corev1.SecretTypeTLS, "": + default: + return fmt.Errorf("cannot use secret '%s' to get TLS certificate: invalid secret type: '%s'", secret.Name, secret.Type) + } + + caFile, ok := secret.Data["ca.crt"] if !ok { - return fmt.Errorf("no caFile found in secret %s", provider.Spec.CertSecretRef.Name) + caFile, ok = secret.Data["caFile"] + if !ok { + return fmt.Errorf("no 'ca.crt' key found in secret '%s'", provider.Spec.CertSecretRef.Name) + } + log.Info("warning: specifying CA cert via 'caFile' is deprecated, please use 'ca.crt' instead") } certPool = x509.NewCertPool() diff --git a/internal/controller/provider_controller_test.go b/internal/controller/provider_controller_test.go index 585e52d07..652156acb 100644 --- a/internal/controller/provider_controller_test.go +++ b/internal/controller/provider_controller_test.go @@ -19,6 +19,7 @@ package controller import ( "context" "fmt" + "os" "testing" "time" @@ -237,3 +238,99 @@ func TestProviderReconciler_Reconcile(t *testing.T) { }, timeout, time.Second).Should(BeTrue()) }) } + +func TestProviderReconciler_Reconcile_cacert(t *testing.T) { + g := NewWithT(t) + namespaceName := "provider-" + randStringRunes(5) + secretName := "ca-secret-" + randStringRunes(5) + + caCrt, err := os.ReadFile("./testdata/certs/ca.pem") + g.Expect(err).To(Not(HaveOccurred())) + + g.Expect(createNamespace(namespaceName)).NotTo(HaveOccurred(), "failed to create test namespace") + + providerKey := types.NamespacedName{ + Name: fmt.Sprintf("provider-%s", randStringRunes(5)), + Namespace: namespaceName, + } + + provider := &apiv1beta2.Provider{ + ObjectMeta: metav1.ObjectMeta{ + Name: providerKey.Name, + Namespace: providerKey.Namespace, + }, + Spec: apiv1beta2.ProviderSpec{ + Type: "generic", + Address: "https://webhook.internal", + CertSecretRef: &meta.LocalObjectReference{Name: secretName}, + }, + } + g.Expect(k8sClient.Create(context.Background(), provider)).To(Succeed()) + + certSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: providerKey.Namespace, + }, + Data: map[string][]byte{ + "caFile": []byte("invalid byte"), + "ca.crt": caCrt, + }, + } + g.Expect(k8sClient.Create(context.Background(), certSecret)).To(Succeed()) + + r := &ProviderReconciler{ + Client: k8sClient, + EventRecorder: record.NewFakeRecorder(32), + } + + t.Run("uses `ca.crt` instead of deprecated `caFile`", func(t *testing.T) { + g := NewWithT(t) + _, err = r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(provider)}) + g.Expect(err).NotTo(HaveOccurred()) + }) + + t.Run("works if only deprecated `caFile` is specified", func(t *testing.T) { + g := NewWithT(t) + + clusterCertSecret := &corev1.Secret{} + g.Expect(k8sClient.Get(context.Background(), client.ObjectKeyFromObject(certSecret), clusterCertSecret)).To(Succeed()) + + patchHelper, err := patch.NewHelper(clusterCertSecret, k8sClient) + g.Expect(err).ToNot(HaveOccurred()) + clusterCertSecret.Data = map[string][]byte{ + "caFile": caCrt, + } + g.Expect(patchHelper.Patch(context.Background(), clusterCertSecret)).ToNot(HaveOccurred()) + + _, err = r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(provider)}) + g.Expect(err).NotTo(HaveOccurred()) + }) + + t.Run("returns error with certSecretRef of the wrong type", func(t *testing.T) { + g := NewWithT(t) + + dockerSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "docker-secret", + Namespace: providerKey.Namespace, + }, + Type: corev1.DockerConfigJsonKey, + } + g.Expect(k8sClient.Create(context.Background(), dockerSecret)).To(Succeed()) + + clusterProvider := &apiv1beta2.Provider{} + g.Expect(k8sClient.Get(context.Background(), client.ObjectKeyFromObject(provider), clusterProvider)).To(Succeed()) + + patchHelper, err := patch.NewHelper(clusterProvider, k8sClient) + g.Expect(err).ToNot(HaveOccurred()) + clusterProvider.Spec.CertSecretRef = &meta.LocalObjectReference{ + Name: dockerSecret.Name, + } + g.Expect(patchHelper.Patch(context.Background(), clusterProvider)).ToNot(HaveOccurred()) + + _, err = r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(provider)}) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("invalid secret type")) + }) +} diff --git a/internal/controller/testdata/certs/Makefile b/internal/controller/testdata/certs/Makefile new file mode 100644 index 000000000..2863849d4 --- /dev/null +++ b/internal/controller/testdata/certs/Makefile @@ -0,0 +1,20 @@ +# Copyright 2023 The Flux 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. + +all: ca-key.pem + +ca-key.pem: ca-csr.json + cfssl gencert -initca ca-csr.json | cfssljson -bare ca – +ca.pem: ca-key.pem +ca.csr: ca-key.pem diff --git a/internal/controller/testdata/certs/ca-config.json b/internal/controller/testdata/certs/ca-config.json new file mode 100644 index 000000000..a63e0dd23 --- /dev/null +++ b/internal/controller/testdata/certs/ca-config.json @@ -0,0 +1,13 @@ +{ + "signing": { + "default": { + "expiry": "8760h" + }, + "profiles": { + "kubernetes": { + "usages": ["signing", "key encipherment", "server auth", "client auth"], + "expiry": "8760h" + } + } + } +} diff --git a/internal/controller/testdata/certs/ca-csr.json b/internal/controller/testdata/certs/ca-csr.json new file mode 100644 index 000000000..941277bb1 --- /dev/null +++ b/internal/controller/testdata/certs/ca-csr.json @@ -0,0 +1,9 @@ +{ + "CN": "example.com CA", + "hosts": [ + "127.0.0.1", + "localhost", + "example.com", + "www.example.com" + ] +} diff --git a/internal/controller/testdata/certs/ca-key.pem b/internal/controller/testdata/certs/ca-key.pem new file mode 100644 index 000000000..a3b86348a --- /dev/null +++ b/internal/controller/testdata/certs/ca-key.pem @@ -0,0 +1,5 @@ +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIEY+R5pZqRQUvD2wrbL4HFl5IKFY84FEGHG5GHW1EojroAoGCCqGSM49 +AwEHoUQDQgAEDqdWaSYf2JT7L90ywnuhz2BS4zhy+v5juPLpBI0Indo8mHpOw9m+ +LCnMzN2WkcUYt+XLQwhaNNt0RBlfnXRzhw== +-----END EC PRIVATE KEY----- diff --git a/internal/controller/testdata/certs/ca.csr b/internal/controller/testdata/certs/ca.csr new file mode 100644 index 000000000..ee4314138 --- /dev/null +++ b/internal/controller/testdata/certs/ca.csr @@ -0,0 +1,9 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIIBHzCBxgIBADAZMRcwFQYDVQQDEw5leGFtcGxlLmNvbSBDQTBZMBMGByqGSM49 +AgEGCCqGSM49AwEHA0IABA6nVmkmH9iU+y/dMsJ7oc9gUuM4cvr+Y7jy6QSNCJ3a +PJh6TsPZviwpzMzdlpHFGLfly0MIWjTbdEQZX510c4egSzBJBgkqhkiG9w0BCQ4x +PDA6MDgGA1UdEQQxMC+CCWxvY2FsaG9zdIILZXhhbXBsZS5jb22CD3d3dy5leGFt +cGxlLmNvbYcEfwAAATAKBggqhkjOPQQDAgNIADBFAiEAl0UiOdxlEcuKrNAGh/Pv +CFxMyX5shaeAsdGvq/gyXckCIFlTwheOJZZVRRQl9b0l5LUVeJyIH6mnvitFGyQ7 +NRk5 +-----END CERTIFICATE REQUEST----- diff --git a/internal/controller/testdata/certs/ca.pem b/internal/controller/testdata/certs/ca.pem new file mode 100644 index 000000000..1c5fbc3b8 --- /dev/null +++ b/internal/controller/testdata/certs/ca.pem @@ -0,0 +1,11 @@ +-----BEGIN CERTIFICATE----- +MIIBhzCCAS2gAwIBAgIUXe+UdziaK3dAdwRgwpWa0XPkKLIwCgYIKoZIzj0EAwIw +GTEXMBUGA1UEAxMOZXhhbXBsZS5jb20gQ0EwHhcNMjMwODE2MTcwMDAwWhcNMjgw +ODE0MTcwMDAwWjAZMRcwFQYDVQQDEw5leGFtcGxlLmNvbSBDQTBZMBMGByqGSM49 +AgEGCCqGSM49AwEHA0IABA6nVmkmH9iU+y/dMsJ7oc9gUuM4cvr+Y7jy6QSNCJ3a +PJh6TsPZviwpzMzdlpHFGLfly0MIWjTbdEQZX510c4ejUzBRMA4GA1UdDwEB/wQE +AwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBRSCe96Yb+o/q3kuqBp5IMs +uD9N/DAPBgNVHREECDAGhwR/AAABMAoGCCqGSM49BAMCA0gAMEUCIBw52JoEQ/0Z +Bnz4gXlXLXtVX0C4LvcNohdlwSRGHPlYAiEAnVqcT2kxBs2+E3SqJPU3DUM7ZFOO +n3zfiIVinQNlXPY= +-----END CERTIFICATE----- diff --git a/internal/server/event_handlers.go b/internal/server/event_handlers.go index ffba6ce1b..b0a57227f 100644 --- a/internal/server/event_handlers.go +++ b/internal/server/event_handlers.go @@ -205,16 +205,28 @@ func (s *EventServer) handleEvent() func(w http.ResponseWriter, r *http.Request) continue } - caFile, ok := secret.Data["caFile"] - if !ok { - alertLogger.Error(err, "failed to read secret key caFile") + switch secret.Type { + case corev1.SecretTypeOpaque, corev1.SecretTypeTLS, "": + default: + alertLogger.Error(nil, "cannot use secret '%s' to get TLS certificate: invalid secret type: '%s'", + secret.Name, secret.Type) continue } + caFile, ok := secret.Data["ca.crt"] + if !ok { + caFile, ok = secret.Data["caFile"] + if !ok { + alertLogger.Error(nil, "no 'ca.crt' key found in secret '%s'", provider.Spec.CertSecretRef.Name) + continue + } + alertLogger.Info("warning: specifying CA cert via 'caFile' is deprecated, please use 'ca.crt' instead") + } + certPool = x509.NewCertPool() ok = certPool.AppendCertsFromPEM(caFile) if !ok { - alertLogger.Error(err, "could not append to cert pool") + alertLogger.Error(nil, "could not append to cert pool") continue } }