diff --git a/docs/spec/v1beta3/providers.md b/docs/spec/v1beta3/providers.md index 2c97628de..2f8fd38f8 100644 --- a/docs/spec/v1beta3/providers.md +++ b/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 diff --git a/go.mod b/go.mod index 7e2ee1eb2..747dc96bf 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 96feece82..cdbcf7c00 100644 --- a/go.sum +++ b/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= diff --git a/internal/notifier/github_helpers.go b/internal/notifier/github_helpers.go index 933f23a80..709a1bfd5 100644 --- a/internal/notifier/github_helpers.go +++ b/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 diff --git a/internal/server/event_handlers.go b/internal/server/event_handlers.go index 0e50dfbc8..be506f7e2 100644 --- a/internal/server/event_handlers.go +++ b/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,11 +416,11 @@ 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) } @@ -428,17 +428,20 @@ func createNotifier(ctx context.Context, kubeClient client.Client, provider *api } 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 +} diff --git a/internal/server/event_handlers_test.go b/internal/server/event_handlers_test.go index 84a4dce12..c57f12760 100644 --- a/internal/server/event_handlers_test.go +++ b/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) {