Merge pull request #1160 from abhijith-darshan/feat/gh_app_tls

Add support for mTLS to GitHub App transport

Author: matheuscscp

docs/spec/v1beta3/providers.md

@@ -1504,10 +1504,15 @@ stringData:
 #### GitHub App
 
 To use Github App authentication, make sure the GitHub App is registered and
-installed with the necessary permissions and the github app secret is created as
+installed with the necessary permissions and the GitHub app secret is created as
 described
 [here](https://fluxcd.io/flux/components/source/gitrepositories/#github).
 
+**NOTE:** (For GitHub Enterprise Server) If the GitHub Server uses a private CA, the CA certificate can be referenced either via `.spec.certSecretRef`
+as described [here](#certificate-secret-reference) or the CA certificate can be added in the GitHub App secret referenced via `.spec.secretRef`.
+If the `.spec.secretRef` contains `tls.crt`, `tls.key` then mutual TLS configuration will be automatically enabled. Omit these keys if the GitHub server does not support mutual TLS.
+If both secret references are specified, then the CA specified in `.spec.certSecretRef` takes precedence over the CA specified in the GitHub App secret.
+
 #### Setting up a GitHub workflow
 
 To trigger a GitHub Actions workflow when a Flux Kustomization finishes reconciling,
@@ -1802,6 +1807,11 @@ permissions to update the commit status and the github app secret is created as
 described
 [here](https://fluxcd.io/flux/components/source/gitrepositories/#github).
 
+**NOTE:** (For GitHub Enterprise Server) If the GitHub Server uses a private CA, the CA certificate can be referenced either via `.spec.certSecretRef`
+as described [here](#certificate-secret-reference) or the CA certificate can be added in the GitHub App secret referenced via `.spec.secretRef`.
+If the `.spec.secretRef` contains `tls.crt`, `tls.key` then mutual TLS configuration will be automatically enabled. Omit these keys if the GitHub server does not support mutual TLS.
+If both secret references are specified, then the CA specified in `.spec.certSecretRef` takes precedence over the CA specified in the GitHub App secret.
+
 #### GitLab
 
 When `.spec.type` is set to `gitlab`, the referenced secret must contain a key called `token` with the value set to a

go.mod

@@ -21,7 +21,7 @@ require (
 	github.com/fluxcd/pkg/apis/meta v1.18.0
 	github.com/fluxcd/pkg/auth v0.27.0
 	github.com/fluxcd/pkg/cache v0.10.0
-	github.com/fluxcd/pkg/git v0.34.0
+	github.com/fluxcd/pkg/git v0.35.0
 	github.com/fluxcd/pkg/masktoken v0.7.0
 	github.com/fluxcd/pkg/runtime v0.80.0
 	github.com/fluxcd/pkg/ssa v0.51.0

go.sum

@@ -142,8 +142,8 @@ github.com/fluxcd/pkg/auth v0.27.0 h1:DFsizUxt9ZDAc+z7+o7jcbtfaxRH55MRD/wdU4CXNC
 github.com/fluxcd/pkg/auth v0.27.0/go.mod h1:YEAHpBFuW5oLlH9ekuJaQdnJ2Q3A7Ny8kha3WY7QMnY=
 github.com/fluxcd/pkg/cache v0.10.0 h1:M+OGDM4da1cnz7q+sZSBtkBJHpiJsLnKVmR9OdMWxEY=
 github.com/fluxcd/pkg/cache v0.10.0/go.mod h1:pPXRzQUDQagsCniuOolqVhnAkbNgYOg8d2cTliPs7ME=
-github.com/fluxcd/pkg/git v0.34.0 h1:qTViWkfpEDnjzySyKRKliqUeGj/DznqlkmPhaDNIsFY=
-github.com/fluxcd/pkg/git v0.34.0/go.mod h1:F9Asm3MlLW4uZx3FF92+bqho+oktdMdnTn/QmXe56NE=
+github.com/fluxcd/pkg/git v0.35.0 h1:mAauhsdfxNW4yQdXviVlvcN/uCGGG0+6p5D1+HFZI9w=
+github.com/fluxcd/pkg/git v0.35.0/go.mod h1:F9Asm3MlLW4uZx3FF92+bqho+oktdMdnTn/QmXe56NE=
 github.com/fluxcd/pkg/masktoken v0.7.0 h1:pitmyOg2pUVdW+nn2Lk/xqm2TaA08uxvOC0ns3sz6bM=
 github.com/fluxcd/pkg/masktoken v0.7.0/go.mod h1:Lc1uoDjO1GY6+YdkK+ZqqBIBWquyV58nlSJ5S1N1IYU=
 github.com/fluxcd/pkg/runtime v0.80.0 h1:vknT2vdQSGTFnAhz4xGk2ZXUWCrXh3whsISStgA57Go=

internal/notifier/github_helpers.go

@@ -80,6 +80,11 @@ func getRepoInfoAndGithubClient(addr string, token string, tlsConfig *tls.Config
 			return nil, err
 		}
 
+		if tlsConfig != nil {
+			// add TLS config to get installation token
+			githubOpts = append(githubOpts, github.WithTLSConfig(tlsConfig))
+		}
+
 		client, err := github.New(githubOpts...)
 		if err != nil {
 			return nil, err

internal/server/event_handlers.go

@@ -18,6 +18,7 @@ package server
 
 import (
 	"context"
+	"crypto/tls"
 	"errors"
 	"fmt"
 	"net/http"
@@ -307,15 +308,8 @@ func createCommitStatus(ctx context.Context, provider *apiv1beta3.Provider, even
 
 // extractAuthFromSecret processes notification-controller specific keys (address, proxy, headers)
 // then uses runtime/secrets to handle standard authentication keys (token, username, password, etc.).
-func extractAuthFromSecret(ctx context.Context, kubeClient client.Client, provider *apiv1beta3.Provider) ([]notifier.Option, map[string][]byte, error) {
+func extractAuthFromSecret(ctx context.Context, secret *corev1.Secret) ([]notifier.Option, map[string][]byte, error) {
 	options := []notifier.Option{}
-
-	secretName := types.NamespacedName{Namespace: provider.Namespace, Name: provider.Spec.SecretRef.Name}
-	var secret corev1.Secret
-	if err := kubeClient.Get(ctx, secretName, &secret); err != nil {
-		return nil, nil, fmt.Errorf("failed to read secret: %w", err)
-	}
-
 	if val, ok := secret.Data["address"]; ok {
 		if len(val) > 2048 {
 			return nil, nil, fmt.Errorf("invalid address in secret: address exceeds maximum length of %d bytes", 2048)
@@ -325,7 +319,7 @@ func extractAuthFromSecret(ctx context.Context, kubeClient client.Client, provid
 	if val, ok := secret.Data["proxy"]; ok {
 		deprecatedProxy := strings.TrimSpace(string(val))
 		if _, err := url.Parse(deprecatedProxy); err != nil {
-			return nil, nil, fmt.Errorf("invalid 'proxy' in secret '%s'", secretName.String())
+			return nil, nil, fmt.Errorf("invalid 'proxy' in secret '%s/%s'", secret.Namespace, secret.Name)
 		}
 		log.FromContext(ctx).Error(nil, "warning: specifying proxy with 'proxy' key in the referenced secret is deprecated, use spec.proxySecretRef with 'address' key instead. Support for the 'proxy' key will be removed in v1.")
 		options = append(options, notifier.WithProxyURL(deprecatedProxy))
@@ -339,7 +333,7 @@ func extractAuthFromSecret(ctx context.Context, kubeClient client.Client, provid
 		options = append(options, notifier.WithHeaders(headers))
 	}
 
-	authMethods, err := secrets.AuthMethodsFromSecret(ctx, &secret)
+	authMethods, err := secrets.AuthMethodsFromSecret(ctx, secret)
 	if err == nil && authMethods != nil {
 		if authMethods.HasTokenAuth() {
 			options = append(options, notifier.WithToken(string(authMethods.Token)))
@@ -394,9 +388,15 @@ func createNotifier(ctx context.Context, kubeClient client.Client, provider *api
 	webhook := provider.Spec.Address
 	var token string
 	var secretData map[string][]byte
+	var providerCertSecret, providerSecret *corev1.Secret
+	var err error
 
 	if provider.Spec.SecretRef != nil {
-		secretOptions, sData, err := extractAuthFromSecret(ctx, kubeClient, provider)
+		providerSecret, err = getSecret(ctx, kubeClient, provider.Spec.SecretRef.Name, provider.GetNamespace())
+		if err != nil {
+			return nil, "", err
+		}
+		secretOptions, sData, err := extractAuthFromSecret(ctx, providerSecret)
 		if err != nil {
 			return nil, "", err
 		}
@@ -416,29 +416,32 @@ func createNotifier(ctx context.Context, kubeClient client.Client, provider *api
 	}
 
 	if provider.Spec.ProxySecretRef != nil {
-		secretRef := types.NamespacedName{
-			Name:      provider.Spec.ProxySecretRef.Name,
-			Namespace: provider.GetNamespace(),
+		proxySecret, err := getSecret(ctx, kubeClient, provider.Spec.ProxySecretRef.Name, provider.GetNamespace())
+		if err != nil {
+			return nil, "", err
 		}
-		proxyURL, err := secrets.ProxyURLFromSecretRef(ctx, kubeClient, secretRef)
+		proxyURL, err := secrets.ProxyURLFromSecret(ctx, proxySecret)
 		if err != nil {
 			return nil, "", fmt.Errorf("failed to get proxy URL: %w", err)
 		}
 		options = append(options, notifier.WithProxyURL(proxyURL.String()))
 	}
 
 	if provider.Spec.CertSecretRef != nil {
-		secretRef := types.NamespacedName{
-			Name:      provider.Spec.CertSecretRef.Name,
-			Namespace: provider.GetNamespace(),
-		}
-		tlsConfig, err := secrets.TLSConfigFromSecretRef(ctx, kubeClient, secretRef)
+		providerCertSecret, err = getSecret(ctx, kubeClient, provider.Spec.CertSecretRef.Name, provider.GetNamespace())
 		if err != nil {
-			return nil, "", fmt.Errorf("failed to get TLS config: %w", err)
+			return nil, "", err
 		}
-		options = append(options, notifier.WithTLSConfig(tlsConfig))
 	}
 
+	tlsConfig, err := getTLSConfigForProvider(ctx, providerCertSecret, providerSecret, provider.Spec.Type)
+	if err != nil {
+		return nil, "", err
+	}
+
+	if tlsConfig != nil {
+		options = append(options, notifier.WithTLSConfig(tlsConfig))
+	}
 	if webhook != "" {
 		options = append(options, notifier.WithURL(webhook))
 	}
@@ -451,6 +454,34 @@ func createNotifier(ctx context.Context, kubeClient client.Client, provider *api
 	return sender, token, nil
 }
 
+// getTLSConfigForProvider - retrieves the TLS configuration from the provider's certSecretRef or secretRef.
+func getTLSConfigForProvider(ctx context.Context, providerCertSecret, providerSecret *corev1.Secret, providerType string) (tlsConfig *tls.Config, err error) {
+	// providerCertSecret takes precedence over providerSecret as it is explicitly specified for TLS configuration
+	if providerCertSecret != nil {
+		tlsConfig, err = secrets.TLSConfigFromSecret(ctx, providerCertSecret)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get TLS config: %w", err)
+		}
+		return
+	}
+	// if providerCertSecret is not specified, and if the provider is a git provider then
+	// attempt to get TLS config from providerSecret if ca.crt exists
+	if isGitProvider(providerType) && providerSecret != nil {
+		authMethods, err := secrets.AuthMethodsFromSecret(ctx, providerSecret)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get TLS config: %w", err)
+		}
+		// only proceed to create TLS config if ca.crt exists in the secret
+		if authMethods != nil && authMethods.HasTLS() {
+			tlsConfig, err = secrets.TLSConfigFromSecret(ctx, providerSecret)
+			if err != nil {
+				return nil, fmt.Errorf("failed to get TLS config: %w", err)
+			}
+		}
+	}
+	return
+}
+
 // eventMatchesAlertSource returns if a given event matches with the given alert
 // source configuration and severity.
 func (s *EventServer) eventMatchesAlertSource(ctx context.Context, event *eventv1.Event, alert *apiv1beta3.Alert, source apiv1.CrossNamespaceObjectReference) bool {
@@ -608,3 +639,12 @@ func excludeInternalMetadata(event *eventv1.Event) {
 		delete(event.Metadata, key)
 	}
 }
+
+func getSecret(ctx context.Context, c client.Client, name, namespace string) (*corev1.Secret, error) {
+	secret := &corev1.Secret{}
+	ref := types.NamespacedName{Name: name, Namespace: namespace}
+	if err := c.Get(ctx, ref, secret); err != nil {
+		return nil, fmt.Errorf("failed to get secret '%s': %w", ref.String(), err)
+	}
+	return secret, nil
+}

internal/server/event_handlers_test.go

@@ -40,7 +40,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
-	log "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
 	"github.com/fluxcd/pkg/apis/meta"
@@ -1518,6 +1518,133 @@ func Test_excludeInternalMetadata(t *testing.T) {
 	}
 }
 
+func TestGetTLSConfigForProvider(t *testing.T) {
+	g := NewWithT(t)
+	ctx := context.Background()
+
+	// Reuse your existing helper.
+	caCert, clientCert, clientKey := generateTestCertificates(t)
+
+	// Expected TLS pieces.
+	caPool := x509.NewCertPool()
+	caPool.AppendCertsFromPEM(caCert)
+
+	clientCertPair, err := tls.X509KeyPair(clientCert, clientKey)
+	if err != nil {
+		t.Fatalf("failed to create client cert pair: %v", err)
+	}
+
+	getSecret := func(name string, data map[string][]byte) *corev1.Secret {
+		return &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{Name: name},
+			Data:       data,
+		}
+	}
+
+	tests := []struct {
+		name               string
+		providerType       string
+		providerCertSecret *corev1.Secret
+		providerSecret     *corev1.Secret
+		wantErr            bool
+		wantTLSConfig      *tls.Config
+	}{
+		{
+			name:         "no secrets returns nil TLS config",
+			providerType: apiv1beta3.GitHubProvider,
+		},
+		{
+			name:               "providerCertSecret in ca.crt with valid CA",
+			providerType:       apiv1beta3.GitHubProvider,
+			providerCertSecret: getSecret("cert-secret", map[string][]byte{"ca.crt": caCert}),
+			wantTLSConfig:      &tls.Config{RootCAs: caPool},
+		},
+		{
+			name:               "providerCertSecret precedence over providerSecret",
+			providerType:       apiv1beta3.GitHubProvider,
+			providerCertSecret: getSecret("cert-secret", map[string][]byte{"ca.crt": caCert}),
+			// Intentionally invalid providerSecret to prove precedence is honored.
+			providerSecret: getSecret("ignored", map[string][]byte{"ca.crt": []byte("not-a-cert")}),
+			wantTLSConfig:  &tls.Config{RootCAs: caPool},
+		},
+		{
+			name:               "providerCertSecret with invalid CA returns error",
+			providerType:       apiv1beta3.GitHubProvider,
+			providerCertSecret: getSecret("cert-secret", map[string][]byte{"ca.crt": []byte("bogus")}),
+			wantErr:            true,
+		},
+		{
+			name:           "providerSecret in ca.crt with valid CA (git provider)",
+			providerType:   apiv1beta3.GitHubProvider,
+			providerSecret: getSecret("git-secret", map[string][]byte{"ca.crt": caCert}),
+			wantTLSConfig:  &tls.Config{RootCAs: caPool},
+		},
+		{
+			name:           "providerSecret without ca.crt (git provider) returns nil TLS config",
+			providerType:   apiv1beta3.GitHubProvider,
+			providerSecret: getSecret("git-secret-no-ca", map[string][]byte{"foo": []byte("bar")}),
+		},
+		{
+			name:           "providerSecret in ca.crt with invalid CA (git provider) returns error",
+			providerType:   apiv1beta3.GitHubProvider,
+			providerSecret: getSecret("git-secret", map[string][]byte{"ca.crt": []byte("aaa")}),
+			wantErr:        true,
+		},
+		{
+			name:           "providerSecret ignored for non-git provider even if ca.crt present",
+			providerType:   apiv1beta3.SlackProvider,
+			providerSecret: getSecret("non-git", map[string][]byte{"ca.crt": caCert}),
+		},
+		{
+			name:               "providerCertSecret with (mTLS)",
+			providerType:       apiv1beta3.SlackProvider,
+			providerCertSecret: getSecret("cert-secret", map[string][]byte{"ca.crt": caCert, "tls.crt": clientCert, "tls.key": clientKey}),
+			wantTLSConfig:      &tls.Config{RootCAs: caPool, Certificates: []tls.Certificate{clientCertPair}},
+		},
+		{
+			name:           "providerSecret with (mTLS)",
+			providerType:   apiv1beta3.GitHubProvider,
+			providerSecret: getSecret("cert-secret", map[string][]byte{"ca.crt": caCert, "tls.crt": clientCert, "tls.key": clientKey}),
+			wantTLSConfig:  &tls.Config{RootCAs: caPool, Certificates: []tls.Certificate{clientCertPair}},
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := getTLSConfigForProvider(ctx, tt.providerCertSecret, tt.providerSecret, tt.providerType)
+			if tt.wantErr {
+				g.Expect(err).To(HaveOccurred())
+				return
+			}
+			g.Expect(err).ToNot(HaveOccurred())
+
+			if tt.wantTLSConfig == nil {
+				g.Expect(got).To(BeNil(), "expected nil TLS config")
+				return
+			}
+
+			g.Expect(got).ToNot(BeNil(), "expected non-nil TLS config")
+
+			// RootCAs presence matches expectation.
+			if tt.wantTLSConfig.RootCAs != nil {
+				g.Expect(got.RootCAs).ToNot(BeNil())
+			} else {
+				g.Expect(got.RootCAs).To(BeNil())
+			}
+
+			// Certificates (mTLS) presence matches expectation.
+			g.Expect(got.Certificates).To(HaveLen(len(tt.wantTLSConfig.Certificates)))
+			if len(tt.wantTLSConfig.Certificates) > 0 {
+				// Basic sanity: leaf cert is parsable; avoids brittle struct equality.
+				g.Expect(got.Certificates[0].Certificate).ToNot(BeNil())
+				_, parseErr := x509.ParseCertificate(got.Certificates[0].Certificate[0])
+				g.Expect(parseErr).ToNot(HaveOccurred())
+			}
+		})
+	}
+}
+
 // generateTestCertificates generates test certificates for mTLS testing.
 // TODO: Move to pkg/runtime/secrets test helpers after mTLS implementation is complete
 func generateTestCertificates(t *testing.T) (caCert, clientCert, clientKey []byte) {