Merge pull request #434 from fluxcd/tls-secret

imagerepo: adopt Kubernetes style TLS secrets

Author: aryan9600

api/v1beta2/imagerepository_types.go

@@ -63,17 +63,21 @@ type ImageRepositorySpec struct {
 	// +optional
 	ServiceAccountName string `json:"serviceAccountName,omitempty"`
 
-	// CertSecretRef can be given the name of a secret containing
+	// CertSecretRef can be given the name of a Secret containing
 	// either or both of
 	//
-	//  - a PEM-encoded client certificate (`certFile`) and private
-	//  key (`keyFile`);
-	//  - a PEM-encoded CA certificate (`caFile`)
+	// - a PEM-encoded client certificate (`tls.crt`) and private
+	// key (`tls.key`);
+	// - a PEM-encoded CA certificate (`ca.crt`)
 	//
-	//  and whichever are supplied, will be used for connecting to the
-	//  registry. The client cert and key are useful if you are
-	//  authenticating with a certificate; the CA cert is useful if
-	//  you are using a self-signed server certificate.
+	// and whichever are supplied, will be used for connecting to the
+	// registry. The client cert and key are useful if you are
+	// authenticating with a certificate; the CA cert is useful if
+	// you are using a self-signed server certificate. The Secret must
+	// be of type `Opaque` or `kubernetes.io/tls`.
+	//
+	// Note: Support for the `caFile`, `certFile` and `keyFile` keys has
+	// been deprecated.
 	// +optional
 	CertSecretRef *meta.LocalObjectReference `json:"certSecretRef,omitempty"`
 

config/crd/bases/image.toolkit.fluxcd.io_imagerepositories.yaml

@@ -287,13 +287,15 @@ spec:
                 - namespaceSelectors
                 type: object
               certSecretRef:
-                description: "CertSecretRef can be given the name of a secret containing
-                  either or both of \n - a PEM-encoded client certificate (`certFile`)
-                  and private key (`keyFile`); - a PEM-encoded CA certificate (`caFile`)
+                description: "CertSecretRef can be given the name of a Secret containing
+                  either or both of \n - a PEM-encoded client certificate (`tls.crt`)
+                  and private key (`tls.key`); - a PEM-encoded CA certificate (`ca.crt`)
                   \n and whichever are supplied, will be used for connecting to the
                   registry. The client cert and key are useful if you are authenticating
                   with a certificate; the CA cert is useful if you are using a self-signed
-                  server certificate."
+                  server certificate. The Secret must be of type `Opaque` or `kubernetes.io/tls`.
+                  \n Note: Support for the `caFile`, `certFile` and `keyFile` keys
+                  has been deprecated."
                 properties:
                   name:
                     description: Name of the referent.

docs/api/v1beta2/image-reflector.md

@@ -473,17 +473,20 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference
 </td>
 <td>
 <em>(Optional)</em>
-<p>CertSecretRef can be given the name of a secret containing
+<p>CertSecretRef can be given the name of a Secret containing
 either or both of</p>
 <ul>
-<li>a PEM-encoded client certificate (<code>certFile</code>) and private
-key (<code>keyFile</code>);</li>
-<li>a PEM-encoded CA certificate (<code>caFile</code>)</li>
+<li>a PEM-encoded client certificate (<code>tls.crt</code>) and private
+key (<code>tls.key</code>);</li>
+<li>a PEM-encoded CA certificate (<code>ca.crt</code>)</li>
 </ul>
 <p>and whichever are supplied, will be used for connecting to the
 registry. The client cert and key are useful if you are
 authenticating with a certificate; the CA cert is useful if
-you are using a self-signed server certificate.</p>
+you are using a self-signed server certificate. The Secret must
+be of type <code>Opaque</code> or <code>kubernetes.io/tls</code>.</p>
+<p>Note: Support for the <code>caFile</code>, <code>certFile</code> and <code>keyFile</code> keys has
+been deprecated.</p>
 </td>
 </tr>
 <tr>
@@ -658,17 +661,20 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference
 </td>
 <td>
 <em>(Optional)</em>
-<p>CertSecretRef can be given the name of a secret containing
+<p>CertSecretRef can be given the name of a Secret containing
 either or both of</p>
 <ul>
-<li>a PEM-encoded client certificate (<code>certFile</code>) and private
-key (<code>keyFile</code>);</li>
-<li>a PEM-encoded CA certificate (<code>caFile</code>)</li>
+<li>a PEM-encoded client certificate (<code>tls.crt</code>) and private
+key (<code>tls.key</code>);</li>
+<li>a PEM-encoded CA certificate (<code>ca.crt</code>)</li>
 </ul>
 <p>and whichever are supplied, will be used for connecting to the
 registry. The client cert and key are useful if you are
 authenticating with a certificate; the CA cert is useful if
-you are using a self-signed server certificate.</p>
+you are using a self-signed server certificate. The Secret must
+be of type <code>Opaque</code> or <code>kubernetes.io/tls</code>.</p>
+<p>Note: Support for the <code>caFile</code>, <code>certFile</code> and <code>keyFile</code> keys has
+been deprecated.</p>
 </td>
 </tr>
 <tr>

docs/spec/v1beta2/imagerepositories.md

@@ -175,41 +175,60 @@ secret to a ServiceAccount, see [Add image pull secret to service account](https
 
 ### Certificate secret reference
 
-`.spec.certSecretRef` is an optional field to specify a name reference to a
-Secret in the same namespace as the ImageRepository containing TLS certificate
-data. This is for two separate purposes:
-- to provide a client certificate and private key, if you use a certificate to authenticate with the image registry; and,
-- to provide a CA certificate, if the registry uses a self-signed certificate
+`.spec.certSecretRef.name` is an optional field to specify a secret containing
+TLS certificate data. The secret can contain the following keys:
 
-These will often go together in case of self-hosted image registry. 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 typically
-provide an option for PEM output.
+* `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.
 
-Assuming that a certificate file and private key are in files `client.crt` and
-`client.key` respectively, a secret can be created with `kubectl`:
+If the server is using a self-signed certificate and has TLS client
+authentication enabled, all three values are required.
+
+The Secret should be of type `Opaque` or `kubernetes.io/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:
 
 ```sh
-kubectl create secret generic tls-certs \
-  --from-file=certFile=client.crt \
-  --from-file=keyFile=client.key
+flux create secret tls --tls-key-file=client.key --tls-crt-file=client.crt --ca-crt-file=ca.crt
 ```
 
-An [encrypted secret](sops-guide) can also be used; the important bit is that
-the data keys in the secret are `certFile` and `keyFile`.
-
-In case of a CA certificate for the client to use, the data key for it is
-`caFile`. Adapting the previous example, if the certificate is in the file
-`ca.crt`, and the client certificate and key are as before, the whole command
-would be:
+Example usage:
 
-```sh
-kubectl create secret generic tls-certs \
-  --from-file=certFile=client.crt \
-  --from-file=keyFile=client.key \
-  --from-file=caFile=ca.crt
+```yaml
+---
+apiVersion: image.toolkit.fluxcd.io/v1beta2
+kind: ImageRepository
+metadata:
+  name: example
+  namespace: default
+spec:
+  interval: 5m0s
+  url: 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>
 ```
 
+**Warning:** Support for the `caFile`, `certFile` and `keyFile` keys have been
+deprecated. If you have any Secrets using these keys and specified in an
+ImageRepository, the controller will log a deprecation warning.
+
 ### Suspend
 
 `.spec.suspend` is an optional field to suspend the reconciliation of an

internal/controller/imagerepository_controller.go

@@ -384,10 +384,20 @@ func (r *ImageRepositoryReconciler) setAuthOptions(ctx context.Context, obj *ima
 			}
 		}
 
-		tr, err := secret.TransportFromSecret(&certSecret)
+		tr, err := secret.TransportFromKubeTLSSecret(&certSecret)
 		if err != nil {
 			return nil, err
 		}
+		if tr.TLSClientConfig == nil {
+			tr, err = secret.TransportFromSecret(&certSecret)
+			if err != nil {
+				return nil, err
+			}
+			if tr.TLSClientConfig != 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")
+			}
+		}
 		options = append(options, remote.WithTransport(tr))
 	}
 

internal/controller/imagerepository_controller_test.go

@@ -102,6 +102,7 @@ func TestImageRepositoryReconciler_setAuthOptions(t *testing.T) {
 	testImg := "example.com/foo/bar"
 	testSecretName := "test-secret"
 	testTLSSecretName := "test-tls-secret"
+	testDeprecatedTLSSecretName := "test-deprecated-tls-secret"
 	testServiceAccountName := "test-service-account"
 	testNamespace := "test-ns"
 
@@ -132,18 +133,27 @@ func TestImageRepositoryReconciler_setAuthOptions(t *testing.T) {
 	testTLSSecret.Namespace = testNamespace
 	testTLSSecret.Type = corev1.SecretTypeTLS
 	testTLSSecret.Data = map[string][]byte{
+		secret.CACrtKey:         rootCertPEM,
+		corev1.TLSCertKey:       clientCertPEM,
+		corev1.TLSPrivateKeyKey: clientKeyPEM,
+	}
+
+	testDeprecatedTLSSecret := &corev1.Secret{}
+	testDeprecatedTLSSecret.Name = testDeprecatedTLSSecretName
+	testDeprecatedTLSSecret.Namespace = testNamespace
+	testDeprecatedTLSSecret.Type = corev1.SecretTypeTLS
+	testDeprecatedTLSSecret.Data = map[string][]byte{
 		secret.CACert:     rootCertPEM,
 		secret.ClientCert: clientCertPEM,
 		secret.ClientKey:  clientKeyPEM,
 	}
 
-	// Secret with docker config and TLS secrets.
-	testSecretWithTLS := testSecret.DeepCopy()
-	testSecretWithTLS.Data = map[string][]byte{
-		".dockerconfigjson": dockerconfigjson,
-		secret.CACert:       rootCertPEM,
-		secret.ClientCert:   clientCertPEM,
-		secret.ClientKey:    clientKeyPEM,
+	// Docker config secret with TLS data.
+	testDockerCfgSecretWithTLS := testSecret.DeepCopy()
+	testDockerCfgSecretWithTLS.Data = map[string][]byte{
+		secret.CACrtKey:         rootCertPEM,
+		corev1.TLSCertKey:       clientCertPEM,
+		corev1.TLSPrivateKeyKey: clientKeyPEM,
 	}
 
 	// ServiceAccount without image pull secret.
@@ -211,6 +221,16 @@ func TestImageRepositoryReconciler_setAuthOptions(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:     "cert secret ref with existing secret using deprecated keys",
+			mockObjs: []client.Object{testDeprecatedTLSSecret},
+			imageRepoSpec: imagev1.ImageRepositorySpec{
+				Image: testImg,
+				CertSecretRef: &meta.LocalObjectReference{
+					Name: testDeprecatedTLSSecretName,
+				},
+			},
+		},
 		{
 			name: "cert secret ref with non-existing secret",
 			imageRepoSpec: imagev1.ImageRepositorySpec{
@@ -235,17 +255,15 @@ func TestImageRepositoryReconciler_setAuthOptions(t *testing.T) {
 			},
 		},
 		{
-			name:     "same secret ref and cert secret ref",
-			mockObjs: []client.Object{testSecretWithTLS},
+			name:     "cert secret ref of type docker config",
+			mockObjs: []client.Object{testDockerCfgSecretWithTLS},
 			imageRepoSpec: imagev1.ImageRepositorySpec{
 				Image: testImg,
-				SecretRef: &meta.LocalObjectReference{
-					Name: testSecretName,
-				},
 				CertSecretRef: &meta.LocalObjectReference{
 					Name: testSecretName,
 				},
 			},
+			wantErr: true,
 		},
 		{
 			name:     "service account without pull secret",