Skip to content

Commit cb95019

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]> (chore): use runtime/secrets authMethods this commit ensures that GitHubApp secret resolution happens via pkg/runtime/secrets Signed-off-by: abhijith-darshan <[email protected]> (chore): update docs Signed-off-by: abhijith-darshan <[email protected]>
1 parent c2b572b commit cb95019

File tree

5 files changed

+130
-24
lines changed

5 files changed

+130
-24
lines changed

docs/spec/v1/gitrepositories.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,8 @@ same pattern.
357357
- The private key that was generated in the pre-requisites.
358358
- (Optional) GitHub Enterprise Server users can set the base URL to
359359
`http(s)://HOSTNAME/api/v3`.
360+
- (Optional) If GitHub Enterprise Server uses a private CA, include its bundle (root and any intermediates) in `ca.crt`.
361+
If the `ca.crt` is specified, then it will be used for TLS verification for all API / Git over `HTTPS` requests to the GitHub Enterprise Server.
360362

361363
```yaml
362364
apiVersion: v1
@@ -372,6 +374,10 @@ stringData:
372374
...
373375
-----END RSA PRIVATE KEY-----
374376
githubAppBaseURL: "<github-enterprise-api-url>" #optional, required only for GitHub Enterprise Server users
377+
ca.crt: | #optional, for GitHub Enterprise Server users
378+
-----BEGIN CERTIFICATE-----
379+
...
380+
-----END CERTIFICATE-----
375381
```
376382

377383
Alternatively, the Flux CLI can be used to automatically create the secret with

go.mod

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,15 @@ 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
3838
github.com/fluxcd/pkg/lockedfile v0.6.0
3939
github.com/fluxcd/pkg/masktoken v0.7.0
4040
github.com/fluxcd/pkg/oci v0.51.0
41-
github.com/fluxcd/pkg/runtime v0.78.0
41+
github.com/fluxcd/pkg/runtime v0.79.0
4242
github.com/fluxcd/pkg/sourceignore v0.13.0
4343
github.com/fluxcd/pkg/ssh v0.20.0
4444
github.com/fluxcd/pkg/tar v0.13.0

go.sum

Lines changed: 6 additions & 6 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=
@@ -398,8 +398,8 @@ github.com/fluxcd/pkg/masktoken v0.7.0 h1:pitmyOg2pUVdW+nn2Lk/xqm2TaA08uxvOC0ns3
398398
github.com/fluxcd/pkg/masktoken v0.7.0/go.mod h1:Lc1uoDjO1GY6+YdkK+ZqqBIBWquyV58nlSJ5S1N1IYU=
399399
github.com/fluxcd/pkg/oci v0.51.0 h1:9oYnm+T4SCVSBif9gn80ALJkMGSERabVMDJiaMIdr7Y=
400400
github.com/fluxcd/pkg/oci v0.51.0/go.mod h1:5J6IhHoDVYCVeBEC+4E3nPeKh7d0kjJ8IEL6NVCiTx4=
401-
github.com/fluxcd/pkg/runtime v0.78.0 h1:xwNZqnazmgURGuLiHDbzST6BI5K9fvZuNS4eMVY35Es=
402-
github.com/fluxcd/pkg/runtime v0.78.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw=
401+
github.com/fluxcd/pkg/runtime v0.79.0 h1:9tv79EiQDx/QJH9mYDd9kZ9WybCVWBUGoiBHij+eKkc=
402+
github.com/fluxcd/pkg/runtime v0.79.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw=
403403
github.com/fluxcd/pkg/sourceignore v0.13.0 h1:ZvkzX2WsmyZK9cjlqOFFW1onHVzhPZIqDbCh96rPqbU=
404404
github.com/fluxcd/pkg/sourceignore v0.13.0/go.mod h1:Z9H1GoBx0ljOhptnzoV0PL6Nd/UzwKcSphP27lqb4xI=
405405
github.com/fluxcd/pkg/ssh v0.20.0 h1:Ak0laIYIc/L8lEfqls/LDWRW8wYPESGaravQsCRGLb8=

internal/controller/gitrepository_controller.go

Lines changed: 25 additions & 15 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
@@ -717,24 +721,30 @@ func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1
717721
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
718722
return nil, e
719723
}
720-
721724
getCreds = func() (*authutils.GitCredentials, error) {
722-
var opts []github.OptFunc
723-
724-
if len(authData) > 0 {
725-
opts = append(opts, github.WithAppData(authData))
725+
targetURL := fmt.Sprintf("%s://%s", u.Scheme, u.Host)
726+
authMethods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTargetURL(targetURL), secrets.WithTLSSystemCertPool())
727+
if err != nil {
728+
return nil, err
726729
}
730+
var appOpts []github.OptFunc
731+
732+
appOpts = append(appOpts, github.WithAppData(authMethods.GitHubAppData))
727733

728734
if proxyURL != nil {
729-
opts = append(opts, github.WithProxyURL(proxyURL))
735+
appOpts = append(appOpts, github.WithProxyURL(proxyURL))
730736
}
731737

732738
if r.TokenCache != nil {
733-
opts = append(opts, github.WithCache(r.TokenCache, sourcev1.GitRepositoryKind,
739+
appOpts = append(appOpts, github.WithCache(r.TokenCache, sourcev1.GitRepositoryKind,
734740
obj.GetName(), obj.GetNamespace(), cache.OperationReconcile))
735741
}
736742

737-
username, password, err := github.GetCredentials(ctx, opts...)
743+
if authMethods.HasTLS() {
744+
appOpts = append(appOpts, github.WithTLSConfig(authMethods.TLS))
745+
}
746+
747+
username, password, err := github.GetCredentials(ctx, appOpts...)
738748
if err != nil {
739749
return nil, err
740750
}
@@ -771,16 +781,16 @@ func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1
771781
return opts, nil
772782
}
773783

774-
func (r *GitRepositoryReconciler) getSecretData(ctx context.Context, name, namespace string) (map[string][]byte, error) {
784+
func (r *GitRepositoryReconciler) getSecret(ctx context.Context, name, namespace string) (*corev1.Secret, error) {
775785
key := types.NamespacedName{
776786
Namespace: namespace,
777787
Name: name,
778788
}
779-
var secret corev1.Secret
780-
if err := r.Client.Get(ctx, key, &secret); err != nil {
781-
return nil, err
789+
secret := &corev1.Secret{}
790+
if err := r.Client.Get(ctx, key, secret); err != nil {
791+
return nil, fmt.Errorf("failed to get secret '%s/%s': %w", namespace, name, err)
782792
}
783-
return secret.Data, nil
793+
return secret, nil
784794
}
785795

786796
// 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)