fluxcd
/
source-controller
mirror of https://github.com/fluxcd/source-controller.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1220 from fluxcd/fix-helm-tls 

helmrepo: fix Secret type check for TLS via `.spec.secretRef`
This commit is contained in:
Sanskar Jaiswal 2023-09-07 13:18:22 +05:30 committed by GitHub
parent ec6877aeea f787fc7046
commit d516627f08
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 123 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
internal/controller/helmrepository_controller_test.go
View File

@ -506,6 +506,38 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
t.Expect(artifact.Revision).ToNot(BeEmpty())
},
},
{
// Regression test for: https://github.com/fluxcd/source-controller/issues/1218
name: "HTTPS with docker config secretRef and caFile key makes ArtifactOutdated=True",
protocol: "https",
server: options{
publicKey: tlsPublicKey,
privateKey: tlsPrivateKey,
ca: tlsCA,
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "ca-file",
},
Data: map[string][]byte{
"caFile": tlsCA,
},
Type: corev1.SecretTypeDockerConfigJson,
},
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "ca-file"}
},
want: sreconcile.ResultSuccess,
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"),
},
afterFunc: func(t *WithT, obj *helmv1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
t.Expect(chartRepo.Path).ToNot(BeEmpty())
t.Expect(chartRepo.Index).ToNot(BeNil())
t.Expect(artifact.Revision).ToNot(BeEmpty())
},
},
{
name: "HTTP without secretRef makes ArtifactOutdated=True",
protocol: "http",
@ -550,6 +582,38 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
t.Expect(artifact.Revision).ToNot(BeEmpty())
},
},
{
// Regression test for: https://github.com/fluxcd/source-controller/issues/1218
name: "HTTP with docker config secretRef sets Reconciling=True",
protocol: "http",
server: options{
username: "git",
password: "1234",
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "basic-auth",
},
Data: map[string][]byte{
"username": []byte("git"),
"password": []byte("1234"),
},
Type: corev1.SecretTypeDockerConfigJson,
},
beforeFunc: func(t *WithT, obj *helmv1.HelmRepository, rev digest.Digest) {
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "basic-auth"}
},
want: sreconcile.ResultSuccess,
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"),
},
afterFunc: func(t *WithT, obj *helmv1.HelmRepository, artifact sourcev1.Artifact, chartRepo *repository.ChartRepository) {
t.Expect(chartRepo.Path).ToNot(BeEmpty())
t.Expect(chartRepo.Index).ToNot(BeNil())
t.Expect(artifact.Revision).ToNot(BeEmpty())
},
},
{
name: "HTTPS with invalid CAFile in certSecretRef makes FetchFailed=True and returns error",
protocol: "https",

6
internal/helm/getter/client_opts.go
View File

@ -115,10 +115,10 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit
}
hrOpts.GetterOpts = append(hrOpts.GetterOpts, opts...)
// If the TLS config is nil, i.e. one couldn't be constructed using `.spec.certSecretRef`
// then try to use `.spec.secretRef`.
// If the TLS config is nil, i.e. one couldn't be constructed using
// `.spec.certSecretRef`, then try to use `.spec.secretRef`.
if hrOpts.TlsConfig == nil && !ociRepo {
hrOpts.TlsConfig, tlsBytes, err = stls.TLSClientConfigFromSecret(*authSecret, url)
hrOpts.TlsConfig, tlsBytes, err = stls.LegacyTLSClientConfigFromSecret(*authSecret, url)
if err != nil {
return nil, "", fmt.Errorf("failed to construct Helm client's TLS config: %w", err)
}

31
internal/tls/config.go
View File

@ -45,9 +45,10 @@ type TLSBytes struct {
// - ca.crt, for the CA certificate
//
// Secrets with no certificate, private key, AND CA cert are ignored. If only a
// certificate OR private key is found, an error is returned.
// certificate OR private key is found, an error is returned. The Secret type
// can be blank, Opaque or kubernetes.io/tls.
func KubeTLSClientConfigFromSecret(secret corev1.Secret, url string) (*tls.Config, *TLSBytes, error) {
return tlsClientConfigFromSecret(secret, url, true)
return tlsClientConfigFromSecret(secret, url, true, true)
}
// TLSClientConfigFromSecret returns a TLS client config as a `tls.Config`
@ -58,9 +59,23 @@ func KubeTLSClientConfigFromSecret(secret corev1.Secret, url string) (*tls.Confi
// - caFile, for the CA certificate
//
// Secrets with no certificate, private key, AND CA cert are ignored. If only a
// certificate OR private key is found, an error is returned.
// certificate OR private key is found, an error is returned. The Secret type
// can be blank, Opaque or kubernetes.io/tls.
func TLSClientConfigFromSecret(secret corev1.Secret, url string) (*tls.Config, *TLSBytes, error) {
return tlsClientConfigFromSecret(secret, url, false)
return tlsClientConfigFromSecret(secret, url, false, true)
}
// LegacyTLSClientConfigFromSecret returns a TLS client config as a `tls.Config`
// object and in its bytes representation. The secret is expected to have the
// following keys:
// - keyFile, for the private key
// - certFile, for the certificate
// - caFile, for the CA certificate
//
// Secrets with no certificate, private key, AND CA cert are ignored. If only a
// certificate OR private key is found, an error is returned.
func LegacyTLSClientConfigFromSecret(secret corev1.Secret, url string) (*tls.Config, *TLSBytes, error) {
return tlsClientConfigFromSecret(secret, url, false, false)
}
// tlsClientConfigFromSecret attempts to construct and return a TLS client
@ -75,7 +90,12 @@ func TLSClientConfigFromSecret(secret corev1.Secret, url string) (*tls.Config, *
// - 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.
func tlsClientConfigFromSecret(secret corev1.Secret, url string, kubernetesTLSKeys bool) (*tls.Config, *TLSBytes, error) {
//
// checkType is a boolean indicating whether to check the Secret type. If true
// and the Secret's type is not blank, Opaque or kubernetes.io/tls, then an
// error is returned.
func tlsClientConfigFromSecret(secret corev1.Secret, url string, kubernetesTLSKeys bool, checkType bool) (*tls.Config, *TLSBytes, error) {
if checkType {
// 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.
@ -84,6 +104,7 @@ func tlsClientConfigFromSecret(secret corev1.Secret, url string, kubernetesTLSKe
default:
return nil, 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 {

17
internal/tls/config_test.go
View File

@ -39,6 +39,7 @@ func Test_tlsClientConfigFromSecret(t *testing.T) {
secret corev1.Secret
modify func(secret *corev1.Secret)
tlsKeys bool
checkType bool
url string
wantErr bool
wantNil bool
@ -86,11 +87,21 @@ func Test_tlsClientConfigFromSecret(t *testing.T) {
wantNil: true,
},
{
name: "invalid secret type",
secret: corev1.Secret{Type: corev1.SecretTypeDockerConfigJson},
name: "docker config secret with type checking enabled",
secret: tlsSecretFixture,
modify: func(secret *corev1.Secret) { secret.Type = corev1.SecretTypeDockerConfigJson },
tlsKeys: false,
checkType: true,
wantErr: true,
wantNil: true,
},
{
name: "docker config secret with type checking disabled",
secret: tlsSecretFixture,
modify: func(secret *corev1.Secret) { secret.Type = corev1.SecretTypeDockerConfigJson },
tlsKeys: false,
url: "https://example.com",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -100,7 +111,7 @@ func Test_tlsClientConfigFromSecret(t *testing.T) {
tt.modify(secret)
}
tlsConfig, _, err := tlsClientConfigFromSecret(*secret, tt.url, tt.tlsKeys)
tlsConfig, _, err := tlsClientConfigFromSecret(*secret, tt.url, tt.tlsKeys, tt.checkType)
g.Expect(err != nil).To(Equal(tt.wantErr), fmt.Sprintf("expected error: %v, got: %v", tt.wantErr, err))
g.Expect(tlsConfig == nil).To(Equal(tt.wantNil))
if tt.url != "" {