Merge pull request #1647 from dipti-pai/github-app-auth

[RFC-007] Implement GitHub app authentication for git repositories.
2025-01-09 20:20:52 +02:00 · 2025-01-09 20:20:52 +02:00 · fe5af75a3a
parent fe7b1fecba 1ed845928b
commit fe5af75a3a
7 changed files with 255 additions and 24 deletions
--- a/api/v1/condition_types.go
+++ b/api/v1/condition_types.go
@ -111,4 +111,8 @@ const (

 	// InvalidSTSConfigurationReason signals that the STS configurtion is invalid.
 	InvalidSTSConfigurationReason string = "InvalidSTSConfiguration"
+
+	// InvalidProviderConfigurationReason signals that the provider
+	// configuration is invalid.
+	InvalidProviderConfigurationReason string = "InvalidProviderConfiguration"
 )
--- a/api/v1/gitrepository_types.go
+++ b/api/v1/gitrepository_types.go
@ -35,6 +35,10 @@ const (
 	// GitProviderAzure provides support for authentication to azure
 	// repositories using Managed Identity.
 	GitProviderAzure string = "azure"
+
+	// GitProviderGitHub provides support for authentication to git
+	// repositories using GitHub App authentication
+	GitProviderGitHub string = "github"
 )

 const (
@ -88,9 +92,9 @@ type GitRepositorySpec struct {
 	// +optional
 	SecretRef *meta.LocalObjectReference `json:"secretRef,omitempty"`

-	// Provider used for authentication, can be 'azure', 'generic'.
+	// Provider used for authentication, can be 'azure', 'github', 'generic'.
 	// When not specified, defaults to 'generic'.
-	// +kubebuilder:validation:Enum=generic;azure
+	// +kubebuilder:validation:Enum=generic;azure;github
 	// +optional
 	Provider string `json:"provider,omitempty"`

--- a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml
+++ b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml
@ -105,11 +105,12 @@ spec:
                type: string
              provider:
                description: |-
-                  Provider used for authentication, can be 'azure', 'generic'.
+                  Provider used for authentication, can be 'azure', 'github', 'generic'.
                  When not specified, defaults to 'generic'.
                enum:
                - generic
                - azure
+                - github
                type: string
              proxySecretRef:
                description: |-
--- a/docs/api/v1/source.md
+++ b/docs/api/v1/source.md
@ -390,7 +390,7 @@ string
 </td>
 <td>
 <em>(Optional)</em>
-<p>Provider used for authentication, can be &lsquo;azure&rsquo;, &lsquo;generic&rsquo;.
+<p>Provider used for authentication, can be &lsquo;azure&rsquo;, &lsquo;github&rsquo;, &lsquo;generic&rsquo;.
 When not specified, defaults to &lsquo;generic&rsquo;.</p>
 </td>
 </tr>
@ -1730,7 +1730,7 @@ string
 </td>
 <td>
 <em>(Optional)</em>
-<p>Provider used for authentication, can be &lsquo;azure&rsquo;, &lsquo;generic&rsquo;.
+<p>Provider used for authentication, can be &lsquo;azure&rsquo;, &lsquo;github&rsquo;, &lsquo;generic&rsquo;.
 When not specified, defaults to &lsquo;generic&rsquo;.</p>
 </td>
 </tr>
--- a/docs/spec/v1/gitrepositories.md
+++ b/docs/spec/v1/gitrepositories.md
@ -221,6 +221,7 @@ Supported options are:

 - `generic`
 - `azure`
+- `github`

 When provider is not specified, it defaults to `generic` indicating that
 mechanisms using `spec.secretRef` are used for authentication. 
@ -296,6 +297,64 @@ must follow this format:
 ```
 https://dev.azure.com/{your-organization}/{your-project}/_git/{your-repository}
 ```
+#### GitHub
+
+The `github` provider can be used to authenticate to Git repositories using
+[GitHub Apps](https://docs.github.com/en/apps/overview).
+
+##### Pre-requisites
+
+- [Register](https://docs.github.com/en/apps/creating-github-apps/registering-a-github-app/registering-a-github-app)
+  the GitHub App with the necessary permissions and [generate a private
+  key](https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/managing-private-keys-for-github-apps)
+  for the app. 
+
+- [Install](https://docs.github.com/en/apps/using-github-apps/installing-your-own-github-app)
+  the app in the organization/account configuring access to the necessary
+  repositories.
+
+##### Configure GitHub App secret
+
+The GitHub App information is specified in `.spec.secretRef` in the format
+specified below:
+
+- Get the App ID from the app settings page at
+  `https://github.com/settings/apps/<app-name>`. 
+- Get the App Installation ID from the app installations page at
+`https://github.com/settings/installations`. Click the installed app, the URL
+will contain the installation ID
+`https://github.com/settings/installations/<installation-id>`. For
+organizations, the first part of the URL may be different, but it follows the
+same pattern.
+- The private key that was generated in the pre-requisites.
+- (Optional) GitHub Enterprise Server users can set the base URL to
+  `http(s)://HOSTNAME/api/v3`.
+
+```yaml
+apiVersion: v1
+kind: Secret
+metadata:
+  name: github-sa
+type: Opaque
+stringData:
+  githubAppID: "<app-id>"
+  githubAppInstallationID: "<app-installation-id>"
+  githubAppPrivateKey: |
+    -----BEGIN RSA PRIVATE KEY-----
+    ...
+    -----END RSA PRIVATE KEY-----
+  githubAppBaseURL: "<github-enterprise-api-url>" #optional, required only for GitHub Enterprise Server users
+```
+
+Alternatively, the Flux CLI can be used to automatically create the secret with
+the github app authentication information.
+
+```sh
+flux create secret githubapp ghapp-secret \
+    --app-id=1 \
+    --app-installation-id=3 \
+    --app-private-key=~/private-key.pem    
+```

 ### Interval

--- a/internal/controller/gitrepository_controller.go
+++ b/internal/controller/gitrepository_controller.go
@ -28,6 +28,7 @@ import (

 	securejoin "github.com/cyphar/filepath-securejoin"
 	"github.com/fluxcd/pkg/auth/azure"
+	"github.com/fluxcd/pkg/auth/github"
 	"github.com/fluxcd/pkg/runtime/logger"
 	"github.com/go-git/go-git/v5/plumbing/transport"
 	corev1 "k8s.io/api/core/v1"
@ -504,13 +505,8 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch

 	authOpts, err := r.getAuthOpts(ctx, obj, *u)
 	if err != nil {
-		e := serror.NewGeneric(
-			fmt.Errorf("failed to configure authentication options: %w", err),
-			sourcev1.AuthenticationFailedReason,
-		)
-		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
 		// Return error as the world as observed may change
-		return sreconcile.ResultEmpty, e
+		return sreconcile.ResultEmpty, err
 	}

 	// Fetch the included artifact metadata.
@ -637,26 +633,63 @@ func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1
 		var err error
 		authData, err = r.getSecretData(ctx, obj.Spec.SecretRef.Name, obj.GetNamespace())
 		if err != nil {
-			return nil, fmt.Errorf("failed to get secret '%s/%s': %w", obj.GetNamespace(), obj.Spec.SecretRef.Name, err)
+			e := serror.NewGeneric(
+				fmt.Errorf("failed to get secret '%s/%s': %w", obj.GetNamespace(), obj.Spec.SecretRef.Name, err),
+				sourcev1.AuthenticationFailedReason,
+			)
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
+			return nil, e
 		}
 	}

 	// Configure authentication strategy to access the source
 	authOpts, err := git.NewAuthOptions(u, authData)
 	if err != nil {
-		return nil, err
+		e := serror.NewGeneric(
+			fmt.Errorf("failed to configure authentication options: %w", err),
+			sourcev1.AuthenticationFailedReason,
+		)
+		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
+		return nil, e
 	}

 	// Configure provider authentication if specified in spec
-	if obj.GetProvider() == sourcev1.GitProviderAzure {
+	switch obj.GetProvider() {
+	case sourcev1.GitProviderAzure:
 		authOpts.ProviderOpts = &git.ProviderOptions{
-			Name: obj.GetProvider(),
+			Name: sourcev1.GitProviderAzure,
 			AzureOpts: []azure.OptFunc{
 				azure.WithAzureDevOpsScope(),
 			},
 		}
-	}
+	case sourcev1.GitProviderGitHub:
+		// if provider is github, but secret ref is not specified
+		if obj.Spec.SecretRef == nil {
+			e := serror.NewStalling(
+				fmt.Errorf("secretRef with github app data must be specified when provider is set to github"),
+				sourcev1.InvalidProviderConfigurationReason,
+			)
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
+			return nil, e
+		}

+		authOpts.ProviderOpts = &git.ProviderOptions{
+			Name: sourcev1.GitProviderGitHub,
+			GitHubOpts: []github.OptFunc{
+				github.WithAppData(authData),
+			},
+		}
+	default:
+		// analyze secret, if it has github app data, perhaps provider should have been github.
+		if appID := authData[github.AppIDKey]; len(appID) != 0 {
+			e := serror.NewStalling(
+				fmt.Errorf("secretRef '%s/%s' has github app data but provider is not set to github", obj.GetNamespace(), obj.Spec.SecretRef.Name),
+				sourcev1.InvalidProviderConfigurationReason,
+			)
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
+			return nil, e
+		}
+	}
 	return authOpts, nil
 }

--- a/internal/controller/gitrepository_controller_test.go
+++ b/internal/controller/gitrepository_controller_test.go
@ -48,6 +48,7 @@ import (

 	kstatus "github.com/fluxcd/cli-utils/pkg/kstatus/status"
 	"github.com/fluxcd/pkg/apis/meta"
+	"github.com/fluxcd/pkg/auth/github"
 	"github.com/fluxcd/pkg/git"
 	"github.com/fluxcd/pkg/gittestserver"
 	"github.com/fluxcd/pkg/runtime/conditions"
@ -571,6 +572,50 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master@sha1:<commit>'"),
 			},
 		},
+		{
+			// This test is only for verifying the failure state when using
+			// provider auth. Protocol http is used for simplicity.
+			name:     "github provider without secret ref makes FetchFailed=True",
+			protocol: "http",
+			beforeFunc: func(obj *sourcev1.GitRepository) {
+				obj.Spec.Provider = sourcev1.GitProviderGitHub
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "foo")
+			},
+			want:    sreconcile.ResultEmpty,
+			wantErr: true,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.InvalidProviderConfigurationReason, "secretRef with github app data must be specified when provider is set to github"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "foo"),
+			},
+		},
+		{
+			// This test is only for verifying the failure state when using
+			// provider auth. Protocol http is used for simplicity.
+			name:     "empty provider with github app data in secret makes FetchFailed=True",
+			protocol: "http",
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "github-app-secret",
+				},
+				Data: map[string][]byte{
+					github.AppIDKey: []byte("1111"),
+				},
+			},
+			beforeFunc: func(obj *sourcev1.GitRepository) {
+				obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "github-app-secret"}
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "foo")
+			},
+			want:    sreconcile.ResultEmpty,
+			wantErr: true,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.InvalidProviderConfigurationReason, "secretRef '/github-app-secret' has github app data but provider is not set to github"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "foo"),
+			},
+		},
 	}

 	for _, tt := range tests {
@ -686,23 +731,88 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 func TestGitRepositoryReconciler_getAuthOpts_provider(t *testing.T) {
 	tests := []struct {
 		name                 string
+		url                  string
+		secret               *corev1.Secret
 		beforeFunc           func(obj *sourcev1.GitRepository)
 		wantProviderOptsName string
+		wantErr              error
 	}{
 		{
 			name: "azure provider",
+			url:  "https://dev.azure.com/foo/bar/_git/baz",
 			beforeFunc: func(obj *sourcev1.GitRepository) {
 				obj.Spec.Provider = sourcev1.GitProviderAzure
 			},
 			wantProviderOptsName: sourcev1.GitProviderAzure,
 		},
+		{
+			name: "github provider with no secret ref",
+			url:  "https://github.com/org/repo.git",
+			beforeFunc: func(obj *sourcev1.GitRepository) {
+				obj.Spec.Provider = sourcev1.GitProviderGitHub
+			},
+			wantProviderOptsName: sourcev1.GitProviderGitHub,
+			wantErr:              errors.New("secretRef with github app data must be specified when provider is set to github"),
+		},
+		{
+			name: "github provider with github app data in secret",
+			url:  "https://example.com/org/repo",
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "githubAppSecret",
+				},
+				Data: map[string][]byte{
+					github.AppIDKey:             []byte("123"),
+					github.AppInstallationIDKey: []byte("456"),
+					github.AppPrivateKey:        []byte("abc"),
+				},
+			},
+			beforeFunc: func(obj *sourcev1.GitRepository) {
+				obj.Spec.Provider = sourcev1.GitProviderGitHub
+				obj.Spec.SecretRef = &meta.LocalObjectReference{
+					Name: "githubAppSecret",
+				}
+			},
+			wantProviderOptsName: sourcev1.GitProviderGitHub,
+		},
+		{
+			name: "generic provider with github app data in secret",
+			url:  "https://example.com/org/repo",
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "githubAppSecret",
+				},
+				Data: map[string][]byte{
+					github.AppIDKey: []byte("123"),
+				},
+			},
+			beforeFunc: func(obj *sourcev1.GitRepository) {
+				obj.Spec.Provider = sourcev1.GitProviderGeneric
+				obj.Spec.SecretRef = &meta.LocalObjectReference{
+					Name: "githubAppSecret",
+				}
+			},
+			wantErr: errors.New("secretRef '/githubAppSecret' has github app data but provider is not set to github"),
+		},
 		{
 			name: "generic provider",
+			url:  "https://example.com/org/repo",
 			beforeFunc: func(obj *sourcev1.GitRepository) {
 				obj.Spec.Provider = sourcev1.GitProviderGeneric
 			},
 		},
 		{
+			name: "secret ref defined for non existing secret",
+			url:  "https://github.com/org/repo.git",
+			beforeFunc: func(obj *sourcev1.GitRepository) {
+				obj.Spec.SecretRef = &meta.LocalObjectReference{
+					Name: "authSecret",
+				}
+			},
+			wantErr: errors.New("failed to get secret '/authSecret': secrets \"authSecret\" not found"),
+		},
+		{
+			url:  "https://example.com/org/repo",
 			name: "no provider",
 		},
 	}
@ -710,22 +820,42 @@ func TestGitRepositoryReconciler_getAuthOpts_provider(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
+			clientBuilder := fakeclient.NewClientBuilder().
+				WithScheme(testEnv.GetScheme()).
+				WithStatusSubresource(&sourcev1.GitRepository{})
+
+			if tt.secret != nil {
+				clientBuilder.WithObjects(tt.secret)
+			}
+
 			obj := &sourcev1.GitRepository{}
-			r := &GitRepositoryReconciler{}
-			url, _ := url.Parse("https://dev.azure.com/foo/bar/_git/baz")
+			r := &GitRepositoryReconciler{
+				EventRecorder: record.NewFakeRecorder(32),
+				Client:        clientBuilder.Build(),
+				features:      features.FeatureGates(),
+				patchOptions:  getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
+			}
+
+			url, err := url.Parse(tt.url)
+			g.Expect(err).ToNot(HaveOccurred())

 			if tt.beforeFunc != nil {
 				tt.beforeFunc(obj)
 			}
 			opts, err := r.getAuthOpts(context.TODO(), obj, *url)

-			g.Expect(err).ToNot(HaveOccurred())
-			g.Expect(opts).ToNot(BeNil())
-			if tt.wantProviderOptsName != "" {
-				g.Expect(opts.ProviderOpts).ToNot(BeNil())
-				g.Expect(opts.ProviderOpts.Name).To(Equal(tt.wantProviderOptsName))
+			if tt.wantErr != nil {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr.Error()))
 			} else {
-				g.Expect(opts.ProviderOpts).To(BeNil())
+				g.Expect(err).ToNot(HaveOccurred())
+				g.Expect(opts).ToNot(BeNil())
+				if tt.wantProviderOptsName != "" {
+					g.Expect(opts.ProviderOpts).ToNot(BeNil())
+					g.Expect(opts.ProviderOpts.Name).To(Equal(tt.wantProviderOptsName))
+				} else {
+					g.Expect(opts.ProviderOpts).To(BeNil())
+				}
 			}
 		})
 	}