Skip to content

Commit

Permalink
imaegrepo: 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 23, 2023
1 parent e2f5bb5 commit 02eb70a
Show file tree
Hide file tree
Showing 6 changed files with 176 additions and 59 deletions.
20 changes: 12 additions & 8 deletions api/v1beta2/imagerepository_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand Down
10 changes: 6 additions & 4 deletions config/crd/bases/image.toolkit.fluxcd.io_imagerepositories.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
26 changes: 16 additions & 10 deletions docs/api/v1beta2/image-reflector.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
12 changes: 11 additions & 1 deletion internal/controller/imagerepository_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand Down
42 changes: 30 additions & 12 deletions internal/controller/imagerepository_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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{
Expand All @@ -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",
Expand Down
125 changes: 101 additions & 24 deletions internal/secret/secret.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,51 +34,128 @@ import (
"k8s.io/apimachinery/pkg/types"
)

// These are intended to match the keys used in e.g.,
// https://github.com/fluxcd/flux2/blob/main/cmd/flux/create_secret_helm.go,
// for consistency (and perhaps this will have its own flux create
// secret subcommand at some point).
const (
ClientCert = "certFile"
ClientKey = "keyFile"
CACert = "caFile"
CACrtKey = "ca.crt"
)

type dockerConfig struct {
Auths map[string]authn.AuthConfig
}

// TransportFromSecret reads the TLS data specified in the provided Secret
// and returns a transport configured with the appropriate TLS settings.
// It checks for the following keys in the Secret:
// - `caFile`, for the CA certificate
// - `certFile` and `keyFile`, for the certificate and private key
//
// If none of these keys exists in the Secret then an empty transport is
// returned. If only a certificate OR private key is found, an error is
// returned.
func TransportFromSecret(certSecret *corev1.Secret) (*http.Transport, error) {
// It's possible the secret doesn't contain any certs after
// all and the default transport could be used; but it's
// simpler here to assume a fresh transport is needed.
transport := &http.Transport{
TLSClientConfig: &tls.Config{},
transport := &http.Transport{}
config, err := tlsConfigFromSecret(certSecret, false)
if err != nil {
return nil, err
}
tlsConfig := transport.TLSClientConfig

if clientCert, ok := certSecret.Data[ClientCert]; ok {
// parse and set client cert and secret
if clientKey, ok := certSecret.Data[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("client certificate found, but no key")
}
if config != nil {
transport.TLSClientConfig = config
}

return transport, nil
}

// TransportFromKubeTLSSecret reads the TLS data specified in the provided
// Secret and returns a transport configured with the appropriate TLS settings.
// It checks for the following keys in the Secret:
// - `ca.crt`, for the CA certificate
// - `tls.crt` and `tls.key`, for the certificate and private key
//
// If none of these keys exists in the Secret then an empty transport is
// returned. If only a certificate OR private key is found, an error is
// returned.
func TransportFromKubeTLSSecret(certSecret *corev1.Secret) (*http.Transport, error) {
// It's possible the secret doesn't contain any certs after
// all and the default transport could be used; but it's
// simpler here to assume a fresh transport is needed.
transport := &http.Transport{}
config, err := tlsConfigFromSecret(certSecret, true)
if err != nil {
return nil, err
}
if config != nil {
transport.TLSClientConfig = config
}

return transport, nil
}

// tlsClientConfigFromSecret attempts to construct and return a TLS client
// config from the given Secret. If the Secret does not contain any TLS
// data, it returns nil.
//
// kubernetesTLSKeys is a boolean indicating whether to check the Secret
// for keys expected to be present in a Kubernetes TLS Secret. Based on its
// value, the Secret is checked for the following keys:
// - tls.key/keyFile for the private key
// - tls.crt/certFile for the certificate
// - ca.crt/caFile for the CA certificate
// The keys should adhere to a single convention, i.e. a Secret with tls.key
// and certFile is invalid.
// Copied from: https://github.com/fluxcd/source-controller/blob/052221c3d8a3ce5fd1a1328db4cc27d31bfd5e59/internal/tls/config.go#L78
func tlsConfigFromSecret(secret *corev1.Secret, kubernetesTLSKeys bool) (*tls.Config, error) {
// Only Secrets of type Opaque and TLS are allowed. We also allow Secrets with a blank
// type, to avoid having to specify the type of the Secret for every test case.
// Since a real Kubernetes Secret is of type Opaque by default, its safe to allow this.
switch secret.Type {
case corev1.SecretTypeOpaque, corev1.SecretTypeTLS, "":
default:
return nil, fmt.Errorf("cannot use secret '%s' to construct TLS config: invalid secret type: '%s'", secret.Name, secret.Type)
}

var certBytes, keyBytes, caBytes []byte
if kubernetesTLSKeys {
certBytes, keyBytes, caBytes = secret.Data[corev1.TLSCertKey], secret.Data[corev1.TLSPrivateKeyKey], secret.Data[CACrtKey]
} else {
certBytes, keyBytes, caBytes = secret.Data[ClientCert], secret.Data[ClientKey], secret.Data[CACert]
}

switch {
case len(certBytes)+len(keyBytes)+len(caBytes) == 0:
return nil, nil
case (len(certBytes) > 0 && len(keyBytes) == 0) || (len(keyBytes) > 0 && len(certBytes) == 0):
return nil, fmt.Errorf("invalid '%s' secret data: both certificate and private key need to be provided",
secret.Name)
}
if caCert, ok := certSecret.Data[CACert]; ok {
syscerts, err := x509.SystemCertPool()

tlsConf := &tls.Config{}
if len(certBytes) > 0 && len(keyBytes) > 0 {
cert, err := tls.X509KeyPair(certBytes, keyBytes)
if err != nil {
return nil, err
}
syscerts.AppendCertsFromPEM(caCert)
tlsConfig.RootCAs = syscerts
tlsConf.Certificates = append(tlsConf.Certificates, cert)
}

return transport, nil
if len(caBytes) > 0 {
cp, err := x509.SystemCertPool()
if err != nil {
return nil, fmt.Errorf("cannot retrieve system certificate pool: %w", err)
}
if !cp.AppendCertsFromPEM(caBytes) {
return nil, fmt.Errorf("cannot append certificate into certificate pool: invalid CA certificate")
}

tlsConf.RootCAs = cp
}

return tlsConf, nil

}

// authFromSecret creates an Authenticator that can be given to the
Expand Down

0 comments on commit 02eb70a

Please sign in to comment.