Skip to content

Commit

Permalink
ocirepo: adopt Kubernetes style TLS secrets for .spec.certSecretRef
Browse files Browse the repository at this point in the history 
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]>
  • Loading branch information
@aryan9600
aryan9600 committed Aug 22, 2023
1 parent 6f205ca commit ac494da
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 70 deletions.
20 changes: 12 additions & 8 deletions api/v1beta2/ocirepository_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,21 @@ type OCIRepositorySpec 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 have
// been deprecated.
// +optional
CertSecretRef *meta.LocalObjectReference `json:"certSecretRef,omitempty"`

Expand Down
10 changes: 6 additions & 4 deletions config/crd/bases/source.toolkit.fluxcd.io_ocirepositories.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,15 @@ 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`)
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
have been deprecated."
properties:
name:
description: Name of the referent.
Expand Down
26 changes: 16 additions & 10 deletions docs/api/v1beta2/source.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1119,17 +1119,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 have
been deprecated.</p>
</td>
</tr>
<tr>
Expand Down Expand Up @@ -3024,17 +3027,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 have
been deprecated.</p>
</td>
</tr>
<tr>
Expand Down
72 changes: 46 additions & 26 deletions docs/spec/v1beta2/ocirepositories.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 `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:

```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>
```

**Warning:** 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)
Expand Down
32 changes: 12 additions & 20 deletions internal/controller/ocirepository_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ package controller

import (
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}

Expand Down
55 changes: 53 additions & 2 deletions internal/controller/ocirepository_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 '<revision>' for '<url>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<revision>' for '<url>'"),
},
},
{
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",
Expand Down Expand Up @@ -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 '<revision>' for '<url>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<revision>' for '<url>'"),
},
},
{
Expand Down Expand Up @@ -1257,7 +1308,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
Generation: 1,
},
Data: map[string][]byte{
"caFile": tlsCA,
"ca.crt": tlsCA,
},
}

Expand Down

0 comments on commit ac494da

Please sign in to comment.