Skip to content

Commit 6dab2c8

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]> (chore): adds github app data check this commit ensures that when provider is github and no github app data is present in the secret, it will error out with invalid configuration Signed-off-by: abhijith-darshan <[email protected]>
1 parent c2b572b commit 6dab2c8

File tree

5 files changed

+138
-24
lines changed

5 files changed

+138
-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: 33 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,38 @@ 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-
724+
targetURL := fmt.Sprintf("%s://%s", u.Scheme, u.Host)
725+
authMethods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTargetURL(targetURL), secrets.WithTLSSystemCertPool())
726+
if err != nil {
727+
return nil, err
728+
}
729+
if !authMethods.HasGitHubAppData() {
730+
e := serror.NewStalling(
731+
fmt.Errorf("secretRef with github app data must be specified when provider is set to github"),
732+
sourcev1.InvalidProviderConfigurationReason,
733+
)
734+
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
735+
return nil, e
736+
}
721737
getCreds = func() (*authutils.GitCredentials, error) {
722-
var opts []github.OptFunc
738+
var appOpts []github.OptFunc
723739

724-
if len(authData) > 0 {
725-
opts = append(opts, github.WithAppData(authData))
726-
}
740+
appOpts = append(appOpts, github.WithAppData(authMethods.GitHubAppData))
727741

728742
if proxyURL != nil {
729-
opts = append(opts, github.WithProxyURL(proxyURL))
743+
appOpts = append(appOpts, github.WithProxyURL(proxyURL))
730744
}
731745

732746
if r.TokenCache != nil {
733-
opts = append(opts, github.WithCache(r.TokenCache, sourcev1.GitRepositoryKind,
747+
appOpts = append(appOpts, github.WithCache(r.TokenCache, sourcev1.GitRepositoryKind,
734748
obj.GetName(), obj.GetNamespace(), cache.OperationReconcile))
735749
}
736750

737-
username, password, err := github.GetCredentials(ctx, opts...)
751+
if authMethods.HasTLS() {
752+
appOpts = append(appOpts, github.WithTLSConfig(authMethods.TLS))
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)