Skip to content

Commit 371c49f

Browse files
(chore): adds tls config for GitHub App auth
this commit ensures that if ca.crt or caFile is available in the github app secret, a tls config with user provided certs is appended to system cert pool and passed to the underlying http transport Signed-off-by: abhijith-darshan <[email protected]> (chore): update target URL for TLSConfigFromSecret this commit ensures that the target URL for runtime/secrets.TLSConfigFromSecret has the scheme and host Signed-off-by: abhijith-darshan <[email protected]> (chore): adds test scenarios this commit adds test scenarios for mTLS GitHub app in reconcile source auth strategy Signed-off-by: abhijith-darshan <[email protected]>
1 parent c2b572b commit 371c49f

File tree

4 files changed

+126
-18
lines changed

4 files changed

+126
-18
lines changed

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ require (
3030
github.com/fluxcd/pkg/apis/meta v1.18.0
3131
github.com/fluxcd/pkg/auth v0.21.0
3232
github.com/fluxcd/pkg/cache v0.10.0
33-
github.com/fluxcd/pkg/git v0.34.0
34-
github.com/fluxcd/pkg/git/gogit v0.37.0
33+
github.com/fluxcd/pkg/git v0.35.0
34+
github.com/fluxcd/pkg/git/gogit v0.38.0
3535
github.com/fluxcd/pkg/gittestserver v0.18.0
3636
github.com/fluxcd/pkg/helmtestserver v0.26.0
3737
github.com/fluxcd/pkg/http/transport v0.6.0

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -382,10 +382,10 @@ github.com/fluxcd/pkg/auth v0.21.0 h1:ckAQqP12wuptXEkMY18SQKWEY09m9e6yI0mEMsDV15
382382
github.com/fluxcd/pkg/auth v0.21.0/go.mod h1:MXmpsXT97c874HCw5hnfqFUP7TsG8/Ss1vFrk8JccfM=
383383
github.com/fluxcd/pkg/cache v0.10.0 h1:M+OGDM4da1cnz7q+sZSBtkBJHpiJsLnKVmR9OdMWxEY=
384384
github.com/fluxcd/pkg/cache v0.10.0/go.mod h1:pPXRzQUDQagsCniuOolqVhnAkbNgYOg8d2cTliPs7ME=
385-
github.com/fluxcd/pkg/git v0.34.0 h1:qTViWkfpEDnjzySyKRKliqUeGj/DznqlkmPhaDNIsFY=
386-
github.com/fluxcd/pkg/git v0.34.0/go.mod h1:F9Asm3MlLW4uZx3FF92+bqho+oktdMdnTn/QmXe56NE=
387-
github.com/fluxcd/pkg/git/gogit v0.37.0 h1:JINylFYpwrxS3MCu5Ei+g6XPgxbs5lv9PppIYYr07KY=
388-
github.com/fluxcd/pkg/git/gogit v0.37.0/go.mod h1:X7YzW5mb4srA05h4SpL2OEGEHq02tbXQF5DPJen9hlc=
385+
github.com/fluxcd/pkg/git v0.35.0 h1:mAauhsdfxNW4yQdXviVlvcN/uCGGG0+6p5D1+HFZI9w=
386+
github.com/fluxcd/pkg/git v0.35.0/go.mod h1:F9Asm3MlLW4uZx3FF92+bqho+oktdMdnTn/QmXe56NE=
387+
github.com/fluxcd/pkg/git/gogit v0.38.0 h1:222KmjpKf9pxqi8rAtm1omDcpGTY4JkahLrAwZ3AcwU=
388+
github.com/fluxcd/pkg/git/gogit v0.38.0/go.mod h1:kHStdfd/AtkH5ED0UEWP2tmMGnfxg1GG92D29M+lRJ0=
389389
github.com/fluxcd/pkg/gittestserver v0.18.0 h1:jkuLmzWFfq+v1ziI0LspZrUzc5WzCO98BaWb8OVRPtk=
390390
github.com/fluxcd/pkg/gittestserver v0.18.0/go.mod h1:2wDLqUkPuixk/8pGQdef9ewaGJXf7Z+xHDVq8PIFG4E=
391391
github.com/fluxcd/pkg/helmtestserver v0.26.0 h1:gKw1MGqWwN94nzs2yg3WKgMxi1RqqlDZXlGziaNCcv4=

internal/controller/gitrepository_controller.go

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
authutils "github.com/fluxcd/pkg/auth/utils"
3232
"github.com/fluxcd/pkg/git/github"
3333
"github.com/fluxcd/pkg/runtime/logger"
34+
"github.com/fluxcd/pkg/runtime/secrets"
3435
"github.com/go-git/go-git/v5/plumbing/transport"
3536
corev1 "k8s.io/api/core/v1"
3637
"k8s.io/apimachinery/pkg/runtime"
@@ -621,10 +622,11 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch
621622
// transport.ProxyOptions object using those settings and then returns it.
622623
func (r *GitRepositoryReconciler) getProxyOpts(ctx context.Context, proxySecretName,
623624
proxySecretNamespace string) (*transport.ProxyOptions, *url.URL, error) {
624-
proxyData, err := r.getSecretData(ctx, proxySecretName, proxySecretNamespace)
625+
secret, err := r.getSecret(ctx, proxySecretName, proxySecretNamespace)
625626
if err != nil {
626627
return nil, nil, fmt.Errorf("failed to get proxy secret '%s/%s': %w", proxySecretNamespace, proxySecretName, err)
627628
}
629+
proxyData := secret.Data
628630
b, ok := proxyData["address"]
629631
if !ok {
630632
return nil, nil, fmt.Errorf("invalid proxy secret '%s/%s': key 'address' is missing", proxySecretNamespace, proxySecretName)
@@ -659,10 +661,11 @@ func (r *GitRepositoryReconciler) getProxyOpts(ctx context.Context, proxySecretN
659661
// URL and returns it.
660662
func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1.GitRepository,
661663
u url.URL, proxyURL *url.URL) (*git.AuthOptions, error) {
664+
var secret *corev1.Secret
662665
var authData map[string][]byte
663666
if obj.Spec.SecretRef != nil {
664667
var err error
665-
authData, err = r.getSecretData(ctx, obj.Spec.SecretRef.Name, obj.GetNamespace())
668+
secret, err = r.getSecret(ctx, obj.Spec.SecretRef.Name, obj.GetNamespace())
666669
if err != nil {
667670
e := serror.NewGeneric(
668671
fmt.Errorf("failed to get secret '%s/%s': %w", obj.GetNamespace(), obj.Spec.SecretRef.Name, err),
@@ -671,6 +674,7 @@ func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1
671674
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
672675
return nil, e
673676
}
677+
authData = secret.Data
674678
}
675679

676680
// Configure authentication strategy to access the source
@@ -719,22 +723,36 @@ func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1
719723
}
720724

721725
getCreds = func() (*authutils.GitCredentials, error) {
722-
var opts []github.OptFunc
726+
var appOpts []github.OptFunc
723727

724728
if len(authData) > 0 {
725-
opts = append(opts, github.WithAppData(authData))
729+
appOpts = append(appOpts, github.WithAppData(authData))
726730
}
727731

728732
if proxyURL != nil {
729-
opts = append(opts, github.WithProxyURL(proxyURL))
733+
appOpts = append(appOpts, github.WithProxyURL(proxyURL))
730734
}
731735

732736
if r.TokenCache != nil {
733-
opts = append(opts, github.WithCache(r.TokenCache, sourcev1.GitRepositoryKind,
737+
appOpts = append(appOpts, github.WithCache(r.TokenCache, sourcev1.GitRepositoryKind,
734738
obj.GetName(), obj.GetNamespace(), cache.OperationReconcile))
735739
}
736740

737-
username, password, err := github.GetCredentials(ctx, opts...)
741+
if len(opts.CAFile) > 0 {
742+
targetURL := fmt.Sprintf("%s://%s", u.Scheme, u.Host)
743+
tlsConfig, err := secrets.TLSConfigFromSecret(ctx, secret, targetURL, secrets.WithSystemCertPool())
744+
if err != nil {
745+
e := serror.NewStalling(
746+
fmt.Errorf("failed to configure TLS from secret '%s/%s': %w", obj.GetNamespace(), obj.Spec.SecretRef.Name, err),
747+
sourcev1.AuthenticationFailedReason,
748+
)
749+
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
750+
return nil, e
751+
}
752+
appOpts = append(appOpts, github.WithTLSConfig(tlsConfig))
753+
}
754+
755+
username, password, err := github.GetCredentials(ctx, appOpts...)
738756
if err != nil {
739757
return nil, err
740758
}
@@ -771,16 +789,16 @@ func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1
771789
return opts, nil
772790
}
773791

774-
func (r *GitRepositoryReconciler) getSecretData(ctx context.Context, name, namespace string) (map[string][]byte, error) {
792+
func (r *GitRepositoryReconciler) getSecret(ctx context.Context, name, namespace string) (*corev1.Secret, error) {
775793
key := types.NamespacedName{
776794
Namespace: namespace,
777795
Name: name,
778796
}
779-
var secret corev1.Secret
780-
if err := r.Client.Get(ctx, key, &secret); err != nil {
781-
return nil, err
797+
secret := &corev1.Secret{}
798+
if err := r.Client.Get(ctx, key, secret); err != nil {
799+
return nil, fmt.Errorf("failed to get secret '%s/%s': %w", namespace, name, err)
782800
}
783-
return secret.Data, nil
801+
return secret, nil
784802
}
785803

786804
// reconcileArtifact archives a new Artifact to the Storage, if the current

internal/controller/gitrepository_controller_test.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package controller
1818

1919
import (
2020
"context"
21+
"encoding/json"
2122
"errors"
2223
"fmt"
2324
"net/http"
@@ -348,6 +349,8 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
348349
server options
349350
secret *corev1.Secret
350351
beforeFunc func(obj *sourcev1.GitRepository)
352+
secretFunc func(secret *corev1.Secret, baseURL string)
353+
middlewareFunc gittestserver.HTTPMiddleware
351354
want sreconcile.Result
352355
wantErr bool
353356
assertConditions []metav1.Condition
@@ -527,6 +530,85 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
527530
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "foo"),
528531
},
529532
},
533+
{
534+
name: "mTLS GitHub App without ca.crt makes FetchFailed=True",
535+
protocol: "https",
536+
server: options{
537+
publicKey: tlsPublicKey,
538+
privateKey: tlsPrivateKey,
539+
ca: tlsCA,
540+
},
541+
secret: &corev1.Secret{
542+
ObjectMeta: metav1.ObjectMeta{Name: "gh-app-no-ca"},
543+
Data: map[string][]byte{
544+
github.KeyAppID: []byte("123"),
545+
github.KeyAppInstallationID: []byte("456"),
546+
github.KeyAppPrivateKey: sshtestdata.PEMBytes["rsa"],
547+
},
548+
},
549+
beforeFunc: func(obj *sourcev1.GitRepository) {
550+
obj.Spec.Provider = sourcev1.GitProviderGitHub
551+
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "gh-app-no-ca"}
552+
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
553+
conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingWithRetryReason, "foo")
554+
},
555+
secretFunc: func(secret *corev1.Secret, baseURL string) {
556+
secret.Data[github.KeyAppBaseURL] = []byte(baseURL + "/api/v3")
557+
},
558+
wantErr: true,
559+
assertConditions: []metav1.Condition{
560+
// should record a FetchFailedCondition due to TLS handshake
561+
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "x509: "),
562+
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
563+
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingWithRetryReason, "foo"),
564+
},
565+
},
566+
{
567+
name: "mTLS GitHub App with ca.crt makes Reconciling=True",
568+
protocol: "https",
569+
server: options{
570+
publicKey: tlsPublicKey,
571+
privateKey: tlsPrivateKey,
572+
ca: tlsCA,
573+
username: github.AccessTokenUsername,
574+
password: "some-enterprise-token",
575+
},
576+
secret: &corev1.Secret{
577+
ObjectMeta: metav1.ObjectMeta{Name: "gh-app-ca"},
578+
Data: map[string][]byte{
579+
github.KeyAppID: []byte("123"),
580+
github.KeyAppInstallationID: []byte("456"),
581+
github.KeyAppPrivateKey: sshtestdata.PEMBytes["rsa"],
582+
},
583+
},
584+
beforeFunc: func(obj *sourcev1.GitRepository) {
585+
obj.Spec.Provider = sourcev1.GitProviderGitHub
586+
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "gh-app-ca"}
587+
},
588+
secretFunc: func(secret *corev1.Secret, baseURL string) {
589+
secret.Data[github.KeyAppBaseURL] = []byte(baseURL + "/api/v3")
590+
secret.Data["ca.crt"] = tlsCA
591+
},
592+
middlewareFunc: func(handler http.Handler) http.Handler {
593+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
594+
if strings.HasPrefix(r.URL.Path, "/api/v3/app/installations/") {
595+
w.WriteHeader(http.StatusOK)
596+
tok := &github.AppToken{
597+
Token: "some-enterprise-token",
598+
ExpiresAt: time.Now().Add(time.Hour),
599+
}
600+
_ = json.NewEncoder(w).Encode(tok)
601+
}
602+
handler.ServeHTTP(w, r)
603+
})
604+
},
605+
wantErr: false,
606+
want: sreconcile.ResultSuccess,
607+
assertConditions: []metav1.Condition{
608+
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master@sha1:<commit>'"),
609+
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master@sha1:<commit>'"),
610+
},
611+
},
530612
// TODO: Add test case for HTTPS with bearer token auth secret. It
531613
// depends on gitkit to have support for bearer token based
532614
// authentication.
@@ -695,6 +777,10 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
695777
defer os.RemoveAll(server.Root())
696778
server.AutoCreate()
697779

780+
if tt.middlewareFunc != nil {
781+
server.AddHTTPMiddlewares(tt.middlewareFunc)
782+
}
783+
698784
repoPath := "/test.git"
699785
localRepo, err := initGitRepo(server, "testdata/git/repository", git.DefaultBranch, repoPath)
700786
g.Expect(err).NotTo(HaveOccurred())
@@ -739,6 +825,10 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
739825
tt.beforeFunc(obj)
740826
}
741827

828+
if tt.secretFunc != nil {
829+
tt.secretFunc(secret, server.HTTPAddress())
830+
}
831+
742832
clientBuilder := fakeclient.NewClientBuilder().
743833
WithScheme(testEnv.GetScheme()).
744834
WithStatusSubresource(&sourcev1.GitRepository{})

0 commit comments

Comments
 (0)