Skip to content

Commit

Permalink
helmrepo: adopt Kubernetes 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.

Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
  • Loading branch information
@aryan9600
aryan9600 committed Aug 22, 2023
1 parent 38f6724 commit 6f205ca
Show file tree
Hide file tree
Showing 11 changed files with 440 additions and 225 deletions.
19 changes: 15 additions & 4 deletions api/v1beta2/helmrepository_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,21 @@ type HelmRepositorySpec struct {
// +optional
SecretRef *meta.LocalObjectReference `json:"secretRef,omitempty"`

// CertSecretRef specifies the Secret containing the TLS authentication
// data. The secret must contain a 'certFile' and 'keyFile', and/or 'caFile'
// fields. It takes precedence over the values specified in the Secret
// referred to by `.spec.secretRef`.
// CertSecretRef can be given the name of a Secret containing
// either or both of
//
// - 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. The Secret must
// be of type `Opaque` or `kubernetes.io/tls`.
//
// It takes precedence over the values specified in the Secret referred
// to by `.spec.secretRef`.
// +optional
CertSecretRef *meta.LocalObjectReference `json:"certSecretRef,omitempty"`

Expand Down
13 changes: 9 additions & 4 deletions config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,15 @@ spec:
- namespaceSelectors
type: object
certSecretRef:
description: CertSecretRef specifies the Secret containing the TLS
authentication data. The secret must contain a 'certFile' and 'keyFile',
and/or 'caFile' fields. It takes precedence over the values specified
in the Secret referred to by `.spec.secretRef`.
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. The Secret must be of type `Opaque` or `kubernetes.io/tls`.
\n It takes precedence over the values specified in the Secret referred
to by `.spec.secretRef`."
properties:
name:
description: Name of the referent.
Expand Down
36 changes: 28 additions & 8 deletions docs/api/v1beta2/source.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -811,10 +811,20 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference
</td>
<td>
<em>(Optional)</em>
<p>CertSecretRef specifies the Secret containing the TLS authentication
data. The secret must contain a &lsquo;certFile&rsquo; and &lsquo;keyFile&rsquo;, and/or &lsquo;caFile&rsquo;
fields. It takes precedence over the values specified in the Secret
referred to by <code>.spec.secretRef</code>.</p>
<p>CertSecretRef can be given the name of a Secret containing
either or both of</p>
<ul>
<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. The Secret must
be of type <code>Opaque</code> or <code>kubernetes.io/tls</code>.</p>
<p>It takes precedence over the values specified in the Secret referred
to by <code>.spec.secretRef</code>.</p>
</td>
</tr>
<tr>
Expand Down Expand Up @@ -2503,10 +2513,20 @@ github.com/fluxcd/pkg/apis/meta.LocalObjectReference
</td>
<td>
<em>(Optional)</em>
<p>CertSecretRef specifies the Secret containing the TLS authentication
data. The secret must contain a &lsquo;certFile&rsquo; and &lsquo;keyFile&rsquo;, and/or &lsquo;caFile&rsquo;
fields. It takes precedence over the values specified in the Secret
referred to by <code>.spec.secretRef</code>.</p>
<p>CertSecretRef can be given the name of a Secret containing
either or both of</p>
<ul>
<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. The Secret must
be of type <code>Opaque</code> or <code>kubernetes.io/tls</code>.</p>
<p>It takes precedence over the values specified in the Secret referred
to by <code>.spec.secretRef</code>.</p>
</td>
</tr>
<tr>
Expand Down
38 changes: 20 additions & 18 deletions docs/spec/v1beta2/helmrepositories.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,32 +467,33 @@ flux create secret oci ghcr-auth \
--password=${GITHUB_PAT}
```

**Note:** Support for specifying TLS authentication data using this API has been
**Warning:** Support for specifying TLS authentication data using this API has been
deprecated. Please use [`.spec.certSecretRef`](#cert-secret-reference) instead.
If the controller uses the secret specfied by this field to configure TLS, then
a deprecation warning will be logged.

### Cert secret reference

`.spec.certSecretRef.name` is an optional field to specify a secret containing TLS
certificate data. The secret can contain the following keys:
`.spec.certSecretRef.name` is an optional field to specify a secret containing
TLS certificate data. The secret can contain the following keys:

* `certFile` and `keyFile`, 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.
* `caFile`, to specify the CA certificate used to verify the server, which is required
if the server is using 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.

If the server is using a self-signed certificate and has TLS client authentication enabled,
all three values are required.
If the server is using a self-signed certificate and has TLS client
authentication enabled, all three values are required.

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 creat secret helm` command:
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
flux create secret helm tls --key-file=client.key --cert-file=client.crt --ca-file=ca.crt
flux create secret tls --tls-key-file=client.key --tls-crt-file=client.crt --ca-crt-file=ca.crt
```

Example usage:
Expand All @@ -515,11 +516,12 @@ kind: Secret
metadata:
name: example-tls
namespace: default
type: kubernetes.io/tls # or Opaque
data:
certFile: <BASE64>
keyFile: <BASE64>
tls.crt: <BASE64>
tls.key: <BASE64>
# NOTE: Can be supplied without the above values
caFile: <BASE64>
ca.crt: <BASE64>
```

### Pass credentials
Expand Down
22 changes: 11 additions & 11 deletions internal/controller/helmchart_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2248,7 +2248,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
registryOpts registryOptions
secretOpts secretOptions
secret *corev1.Secret
certsecret *corev1.Secret
certSecret *corev1.Secret
insecure bool
provider string
providerImg string
Expand Down Expand Up @@ -2363,16 +2363,16 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
Type: corev1.SecretTypeDockerConfigJson,
Data: map[string][]byte{},
},
certsecret: &corev1.Secret{
certSecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "certs-secretref",
},
Data: map[string][]byte{
"caFile": []byte("invalid caFile"),
"ca.crt": []byte("invalid caFile"),
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to construct Helm client's TLS config: cannot append certificate into certificate pool: invalid caFile"),
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "Unknown", "unknown build error: failed to construct Helm client's TLS config: cannot append certificate into certificate pool: invalid CA certificate"),
},
},
{
Expand All @@ -2393,14 +2393,14 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
Type: corev1.SecretTypeDockerConfigJson,
Data: map[string][]byte{},
},
certsecret: &corev1.Secret{
certSecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "certs-secretref",
},
Data: map[string][]byte{
"caFile": tlsCA,
"certFile": clientPublicKey,
"keyFile": clientPrivateKey,
"ca.crt": tlsCA,
"tls.crt": clientPublicKey,
"tls.key": clientPrivateKey,
},
},
assertConditions: []metav1.Condition{
Expand Down Expand Up @@ -2472,11 +2472,11 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
clientBuilder.WithObjects(tt.secret)
}

if tt.certsecret != nil {
if tt.certSecret != nil {
repo.Spec.CertSecretRef = &meta.LocalObjectReference{
Name: tt.certsecret.Name,
Name: tt.certSecret.Name,
}
clientBuilder.WithObjects(tt.certsecret)
clientBuilder.WithObjects(tt.certSecret)
}

clientBuilder.WithObjects(repo)
Expand Down
10 changes: 5 additions & 5 deletions internal/controller/helmrepository_controller_oci_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,12 +325,12 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
Name: "certs-secretref",
},
Data: map[string][]byte{
"caFile": []byte("invalid caFile"),
"ca.crt": []byte("invalid caFile"),
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation 0 -> 1"),
*conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid caFile"),
*conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid CA certificate"),
},
},
{
Expand All @@ -356,9 +356,9 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
Name: "certs-secretref",
},
Data: map[string][]byte{
"caFile": tlsCA,
"certFile": clientPublicKey,
"keyFile": clientPrivateKey,
"ca.crt": tlsCA,
"tls.crt": clientPublicKey,
"tls.key": clientPrivateKey,
},
},
assertConditions: []metav1.Condition{
Expand Down
30 changes: 26 additions & 4 deletions internal/controller/helmrepository_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import (
"github.com/fluxcd/source-controller/internal/helm/repository"
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
"github.com/fluxcd/source-controller/internal/reconcile/summarize"
stls "github.com/fluxcd/source-controller/internal/tls"
)

func TestHelmRepositoryReconciler_deleteBeforeFinalizer(t *testing.T) {
Expand Down Expand Up @@ -434,7 +435,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
Name: "ca-file",
},
Data: map[string][]byte{
"caFile": tlsCA,
"ca.crt": tlsCA,
},
},
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
Expand All @@ -445,6 +446,27 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
},
},
{
name: "HTTPS with secretRef pointing to CA cert but public repo URL succeeds",
protocol: "http",
url: "https://stefanprodan.github.io/podinfo",
want: sreconcile.ResultSuccess,
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "ca-file",
},
Data: map[string][]byte{
"caFile": tlsCA,
},
},
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "ca-file"}
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
},
},
{
name: "HTTP without secretRef makes ArtifactOutdated=True",
protocol: "http",
Expand Down Expand Up @@ -502,7 +524,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
Name: "invalid-ca",
},
Data: map[string][]byte{
"caFile": []byte("invalid"),
"ca.crt": []byte("invalid"),
},
},
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
Expand All @@ -512,7 +534,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
},
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid caFile"),
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "cannot append certificate into certificate pool: invalid CA certificate"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
Expand Down Expand Up @@ -769,7 +791,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
if tt.url != "" {
repoURL = tt.url
}
tlsConf, _, serr = getter.TLSClientConfigFromSecret(*secret, repoURL)
tlsConf, _, serr = stls.KubeTLSClientConfigFromSecret(*secret, repoURL)
if serr != nil {
validSecret = false
}
Expand Down
Loading

0 comments on commit 6f205ca

Please sign in to comment.