ocirepo: adopt Kubernetes style TLS secrets for .spec.certSecretRef

Adopt Kubernetes TLS secrets API to check for TLS data in the Secret referred to by `.spec.certSecretRef`, i.e. check for keys `tls.crt` and `tls.key` for the certificate and private key. Use `ca.crt` for the CA certificate. Deprecate the usage of `caFile`, `certFile` and `keyFile` keys. Signed-off-by: Sanskar Jaiswal <[email protected]>
fluxcd · Aug 14, 2023 · 12781bd · 12781bd
1 parent f408fb7
commit 12781bd
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 49 deletions.
diff --git a/docs/spec/v1beta2/ocirepositories.md b/docs/spec/v1beta2/ocirepositories.md
@@ -300,42 +300,62 @@ fetch the image pull secrets attached to the service account and use them for au
 **Note:** that for a publicly accessible image repository, you don't need to provide a `secretRef`
 nor `serviceAccountName`.
 
-### TLS Certificates
+### Cert secret reference
 
-`.spec.certSecretRef` field names a secret with TLS certificate data. This is for two separate
-purposes:
+`.spec.certSecretRef.name` is an optional field to specify a secret containing
+TLS certificate data. The secret can contain the following keys:
 
-- to provide a client certificate and private key, if you use a certificate to authenticate with
-  the container registry; and,
-- to provide a CA certificate, if the registry uses a self-signed certificate.
+* `tls.crt` and `tls.key`, to specify the client certificate and private key used
+for TLS client authentication. These must be used in conjunction, i.e.
+specifying one without the other will lead to an error.
+* `ca.crt`, to specify the CA certificate used to verify the server, which is
+required if the server is using a self-signed certificate.
 
-These will often go together, if you are hosting a container registry yourself. All the files in the
-secret are expected to be [PEM-encoded][pem-encoding]. This is an ASCII format for certificates and
-keys; `openssl` and such tools will typically give you an option of PEM output.
+If the server is using a self-signed certificate and has TLS client
+authentication enabled, all three values are required.
 
-Assuming you have obtained a certificate file and private key and put them in the files `client.crt`
-and `client.key` respectively, you can create a secret with `kubectl` like this:
+The Secret should be of type Opaque or TLS. All the files in the Secret are
+expected to be [PEM-encoded][pem-encoding]. Assuming you have three files;
+`client.key`, `client.crt` and `ca.crt` for the client private key, client
+certificate and the CA certificate respectively, you can generate the required
+Secret using the `flux create secret tls` command:
 
-```bash
-kubectl create secret generic tls-certs \
-  --from-file=certFile=client.crt \
-  --from-file=keyFile=client.key
+```sh
+flux create secret tls --tls-key-file=client.key --tls-crt-file=client.crt --ca-crt-file=ca.crt
 ```
 
-You could also [prepare a secret and encrypt it][sops-guide]; the important bit is that the data
-keys in the secret are `certFile` and `keyFile`.
-
-If you have a CA certificate for the client to use, the data key for that is `caFile`. Adapting the
-previous example, if you have the certificate in the file `ca.crt`, and the client certificate and
-key as before, the whole command would be:
+Example usage:
 
-```bash
-kubectl create secret generic tls-certs \
-  --from-file=certFile=client.crt \
-  --from-file=keyFile=client.key \
-  --from-file=caFile=ca.crt
+```yaml
+---
+apiVersion: source.toolkit.fluxcd.io/v1beta2
+kind: OCIRepository
+metadata:
+  name: example
+  namespace: default
+spec:
+  interval: 5m0s
+  url: oci://example.com
+  certSecretRef:
+    name: example-tls
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: example-tls
+  namespace: default
+type: kubernetes.io/tls # or Opaque
+data:
+  tls.crt: <BASE64>
+  tls.key: <BASE64>
+  # NOTE: Can be supplied without the above values
+  ca.crt: <BASE64>
 ```
 
+**Note:** Support for the `caFile`, `certFile` and `keyFile` keys have been
+deprecated. If you have any Secrets using these keys and specified in an
+OCIRepository, the controller will log a deprecation warning.
+
 ### Insecure
 
 `.spec.insecure` is an optional field to allow connecting to an insecure (HTTP)

diff --git a/internal/controller/ocirepository_controller.go b/internal/controller/ocirepository_controller.go
@@ -18,8 +18,6 @@ package controller
 
 import (
 	"context"
-	"crypto/tls"
-	"crypto/x509"
 	"errors"
 	"fmt"
 	"io"
@@ -68,6 +66,7 @@ import (
 	sourcev1 "github.com/fluxcd/source-controller/api/v1"
 	ociv1 "github.com/fluxcd/source-controller/api/v1beta2"
 	serror "github.com/fluxcd/source-controller/internal/error"
+	"github.com/fluxcd/source-controller/internal/helm/getter"
 	soci "github.com/fluxcd/source-controller/internal/oci"
 	sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
 	"github.com/fluxcd/source-controller/internal/reconcile/summarize"
@@ -843,29 +842,22 @@ func (r *OCIRepositoryReconciler) transport(ctx context.Context, obj *ociv1.OCIR
 	}
 
 	transport := remote.DefaultTransport.(*http.Transport).Clone()
-	tlsConfig := transport.TLSClientConfig
-
-	if clientCert, ok := certSecret.Data[oci.ClientCert]; ok {
-		// parse and set client cert and secret
-		if clientKey, ok := certSecret.Data[oci.ClientKey]; ok {
-			cert, err := tls.X509KeyPair(clientCert, clientKey)
-			if err != nil {
-				return nil, err
-			}
-			tlsConfig.Certificates = append(tlsConfig.Certificates, cert)
-		} else {
-			return nil, fmt.Errorf("'%s' found in secret, but no %s", oci.ClientCert, oci.ClientKey)
-		}
+	tlsConfig, _, err := getter.TLSClientConfigFromSecret(certSecret, "", true)
+	if err != nil {
+		return nil, err
 	}
-
-	if caCert, ok := certSecret.Data[oci.CACert]; ok {
-		syscerts, err := x509.SystemCertPool()
+	if tlsConfig == nil {
+		tlsConfig, _, err = getter.TLSClientConfigFromSecret(certSecret, "", false)
 		if err != nil {
 			return nil, err
 		}
-		syscerts.AppendCertsFromPEM(caCert)
-		tlsConfig.RootCAs = syscerts
+		if tlsConfig != nil {
+			ctrl.LoggerFrom(ctx).
+				Info("warning: specifying TLS auth data via `certFile`/`keyFile`/`caFile` is deprecated, please use `tls.crt`/`tls.key`/ca.crt instead")
+		}
 	}
+	transport.TLSClientConfig = tlsConfig
+
 	return transport, nil
 }
 

diff --git a/internal/controller/ocirepository_controller_test.go b/internal/controller/ocirepository_controller_test.go
@@ -562,7 +562,7 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
 					Name: "ca-file",
 				},
 				Data: map[string][]byte{
-					"caFile": tlsCA,
+					"ca.crt": tlsCA,
 				},
 			},
 			assertConditions: []metav1.Condition{
@@ -605,11 +605,37 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
 					Name: "ca-file",
 				},
 				Data: map[string][]byte{
+					"ca.crt": []byte("invalid"),
+				},
+			},
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, ociv1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid CA certificate"),
+			},
+		},
+		{
+			name: "HTTPS with certfile using both caFile and ca.crt ignores caFile",
+			want: sreconcile.ResultSuccess,
+			registryOpts: registryOptions{
+				withTLS: true,
+			},
+			craneOpts: []crane.Option{crane.WithTransport(&http.Transport{
+				TLSClientConfig: &tls.Config{
+					RootCAs: pool,
+				},
+			}),
+			},
+			tlsCertSecret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "ca-file",
+				},
+				Data: map[string][]byte{
+					"ca.crt": tlsCA,
 					"caFile": []byte("invalid"),
 				},
 			},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.FetchFailedCondition, ociv1.OCIPullFailedReason, "failed to determine artifact digest"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision '<revision>' for '<url>'"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<revision>' for '<url>'"),
 			},
 		},
 		{
@@ -1257,7 +1283,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
 			Generation: 1,
 		},
 		Data: map[string][]byte{
-			"caFile": tlsCA,
+			"ca.crt": tlsCA,
 		},
 	}