diff --git a/api/v1beta2/ocirepository_types.go b/api/v1beta2/ocirepository_types.go index 9019da519..e6a2ec39f 100644 --- a/api/v1beta2/ocirepository_types.go +++ b/api/v1beta2/ocirepository_types.go @@ -100,14 +100,17 @@ type OCIRepositorySpec struct { // 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. + + // Note: Support for the `caFile`, `certFile` and `keyFile` keys has + // been deprecated. // +optional CertSecretRef *meta.LocalObjectReference `json:"certSecretRef,omitempty"` diff --git a/config/crd/bases/source.toolkit.fluxcd.io_ocirepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_ocirepositories.yaml index 8fd16bf16..ab8b8ff95 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_ocirepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_ocirepositories.yaml @@ -50,13 +50,8 @@ spec: description: OCIRepositorySpec defines the desired state of OCIRepository properties: 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`) - \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." + description: 'Note: Support for the `caFile`, `certFile` and `keyFile` + keys has been deprecated.' properties: name: description: Name of the referent. diff --git a/docs/api/v1beta2/source.md b/docs/api/v1beta2/source.md index be0c454ed..0b9b657e0 100644 --- a/docs/api/v1beta2/source.md +++ b/docs/api/v1beta2/source.md @@ -1109,17 +1109,8 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference (Optional) -

CertSecretRef can be given the name of a secret containing -either or both of

- -

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.

+

Note: Support for the caFile, certFile and keyFile keys has +been deprecated.

@@ -3004,17 +2995,8 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference (Optional) -

CertSecretRef can be given the name of a secret containing -either or both of

- -

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.

+

Note: Support for the caFile, certFile and keyFile keys has +been deprecated.

diff --git a/docs/spec/v1beta2/ocirepositories.md b/docs/spec/v1beta2/ocirepositories.md index d2a4bfe6b..4786e5ee5 100644 --- a/docs/spec/v1beta2/ocirepositories.md +++ b/docs/spec/v1beta2/ocirepositories.md @@ -310,42 +310,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: + tls.key: + # NOTE: Can be supplied without the above values + ca.crt: ``` +**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 index 23939ecb8..f10735408 100644 --- 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" @@ -71,6 +69,7 @@ import ( soci "github.com/fluxcd/source-controller/internal/oci" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" + "github.com/fluxcd/source-controller/internal/tls" "github.com/fluxcd/source-controller/internal/util" ) @@ -841,29 +840,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 := tls.KubeTLSClientConfigFromSecret(certSecret, "") + if err != nil { + return nil, err } - - if caCert, ok := certSecret.Data[oci.CACert]; ok { - syscerts, err := x509.SystemCertPool() + if tlsConfig == nil { + tlsConfig, _, err = tls.TLSClientConfigFromSecret(certSecret, "") 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 index ee8f3af80..30fc10bae 100644 --- a/internal/controller/ocirepository_controller_test.go +++ b/internal/controller/ocirepository_controller_test.go @@ -557,6 +557,31 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) { }, }), }, + tlsCertSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ca-file", + }, + Data: map[string][]byte{ + "ca.crt": tlsCA, + }, + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision '' for ''"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '' for ''"), + }, + }, + { + name: "HTTPS with valid certfile using deprecated keys", + 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", @@ -605,11 +630,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 '' for ''"), + *conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '' for ''"), }, }, { @@ -1257,7 +1308,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) { Generation: 1, }, Data: map[string][]byte{ - "caFile": tlsCA, + "ca.crt": tlsCA, }, }