diff --git a/apps/provisioning/pkg/connection/connection.go b/apps/provisioning/pkg/connection/connection.go index d2043b5af4a..57e6aad9989 100644 --- a/apps/provisioning/pkg/connection/connection.go +++ b/apps/provisioning/pkg/connection/connection.go @@ -2,6 +2,9 @@ package connection import ( "context" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" ) //go:generate mockery --name Connection --structname MockConnection --inpackage --filename connection_mock.go --with-expecter @@ -13,4 +16,9 @@ type Connection interface { // Mutate performs in place mutation of the underneath resource. Mutate(context.Context) error + + // GenerateRepositoryToken generates a repository-scoped access token. + // For GitHub connections, this creates an installation token using the GitHub App credentials. + // The repo parameter specifies the repository name the token should be scoped to. + GenerateRepositoryToken(ctx context.Context, repo *provisioning.Repository) (common.RawSecureValue, error) } diff --git a/apps/provisioning/pkg/connection/connection_mock.go b/apps/provisioning/pkg/connection/connection_mock.go index 3867059d432..8ece7dbf054 100644 --- a/apps/provisioning/pkg/connection/connection_mock.go +++ b/apps/provisioning/pkg/connection/connection_mock.go @@ -5,7 +5,11 @@ package connection import ( context "context" + commonv0alpha1 "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" + mock "github.com/stretchr/testify/mock" + + v0alpha1 "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" ) // MockConnection is an autogenerated mock type for the Connection type @@ -21,6 +25,63 @@ func (_m *MockConnection) EXPECT() *MockConnection_Expecter { return &MockConnection_Expecter{mock: &_m.Mock} } +// GenerateRepositoryToken provides a mock function with given fields: ctx, repo +func (_m *MockConnection) GenerateRepositoryToken(ctx context.Context, repo *v0alpha1.Repository) (commonv0alpha1.RawSecureValue, error) { + ret := _m.Called(ctx, repo) + + if len(ret) == 0 { + panic("no return value specified for GenerateRepositoryToken") + } + + var r0 commonv0alpha1.RawSecureValue + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *v0alpha1.Repository) (commonv0alpha1.RawSecureValue, error)); ok { + return rf(ctx, repo) + } + if rf, ok := ret.Get(0).(func(context.Context, *v0alpha1.Repository) commonv0alpha1.RawSecureValue); ok { + r0 = rf(ctx, repo) + } else { + r0 = ret.Get(0).(commonv0alpha1.RawSecureValue) + } + + if rf, ok := ret.Get(1).(func(context.Context, *v0alpha1.Repository) error); ok { + r1 = rf(ctx, repo) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockConnection_GenerateRepositoryToken_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'GenerateRepositoryToken' +type MockConnection_GenerateRepositoryToken_Call struct { + *mock.Call +} + +// GenerateRepositoryToken is a helper method to define mock.On call +// - ctx context.Context +// - repo *v0alpha1.Repository +func (_e *MockConnection_Expecter) GenerateRepositoryToken(ctx interface{}, repo interface{}) *MockConnection_GenerateRepositoryToken_Call { + return &MockConnection_GenerateRepositoryToken_Call{Call: _e.mock.On("GenerateRepositoryToken", ctx, repo)} +} + +func (_c *MockConnection_GenerateRepositoryToken_Call) Run(run func(ctx context.Context, repo *v0alpha1.Repository)) *MockConnection_GenerateRepositoryToken_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*v0alpha1.Repository)) + }) + return _c +} + +func (_c *MockConnection_GenerateRepositoryToken_Call) Return(_a0 commonv0alpha1.RawSecureValue, _a1 error) *MockConnection_GenerateRepositoryToken_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockConnection_GenerateRepositoryToken_Call) RunAndReturn(run func(context.Context, *v0alpha1.Repository) (commonv0alpha1.RawSecureValue, error)) *MockConnection_GenerateRepositoryToken_Call { + _c.Call.Return(run) + return _c +} + // Mutate provides a mock function with given fields: _a0 func (_m *MockConnection) Mutate(_a0 context.Context) error { ret := _m.Called(_a0) diff --git a/apps/provisioning/pkg/connection/factory_test.go b/apps/provisioning/pkg/connection/factory_test.go index 4ce6bc96e0e..150e0e66d89 100644 --- a/apps/provisioning/pkg/connection/factory_test.go +++ b/apps/provisioning/pkg/connection/factory_test.go @@ -1,4 +1,4 @@ -package connection +package connection_test import ( "context" @@ -6,304 +6,319 @@ import ( "testing" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/apps/provisioning/pkg/connection" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestProvideFactory(t *testing.T) { - t.Run("should create factory with valid extras", func(t *testing.T) { - extra1 := NewMockExtra(t) - extra1.EXPECT().Type().Return(provisioning.GithubConnectionType) + tests := []struct { + name string + setupExtras func(t *testing.T) []connection.Extra + enabled map[provisioning.ConnectionType]struct{} + wantErr bool + validateError func(t *testing.T, err error) + }{ + { + name: "should create factory with valid extras", + setupExtras: func(t *testing.T) []connection.Extra { + extra1 := connection.NewMockExtra(t) + extra1.EXPECT().Type().Return(provisioning.GithubConnectionType) - extra2 := NewMockExtra(t) - extra2.EXPECT().Type().Return(provisioning.GitlabConnectionType) + extra2 := connection.NewMockExtra(t) + extra2.EXPECT().Type().Return(provisioning.GitlabConnectionType) - enabled := map[provisioning.ConnectionType]struct{}{ - provisioning.GithubConnectionType: {}, - provisioning.GitlabConnectionType: {}, - } + return []connection.Extra{extra1, extra2} + }, + enabled: map[provisioning.ConnectionType]struct{}{ + provisioning.GithubConnectionType: {}, + provisioning.GitlabConnectionType: {}, + }, + wantErr: false, + }, + { + name: "should return error when duplicate connection types", + setupExtras: func(t *testing.T) []connection.Extra { + extra1 := connection.NewMockExtra(t) + extra1.EXPECT().Type().Return(provisioning.GithubConnectionType) - factory, err := ProvideFactory(enabled, []Extra{extra1, extra2}) - require.NoError(t, err) - require.NotNil(t, factory) - }) + extra2 := connection.NewMockExtra(t) + extra2.EXPECT().Type().Return(provisioning.GithubConnectionType) - t.Run("should create factory with empty extras", func(t *testing.T) { - enabled := map[provisioning.ConnectionType]struct{}{} + return []connection.Extra{extra1, extra2} + }, + enabled: map[provisioning.ConnectionType]struct{}{ + provisioning.GithubConnectionType: {}, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + assert.Contains(t, err.Error(), "connection type \"github\" is already registered") + }, + }, + } - factory, err := ProvideFactory(enabled, []Extra{}) - require.NoError(t, err) - require.NotNil(t, factory) - }) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + extras := tt.setupExtras(t) - t.Run("should create factory with nil enabled map", func(t *testing.T) { - extra1 := NewMockExtra(t) - extra1.EXPECT().Type().Return(provisioning.GithubConnectionType) + factory, err := connection.ProvideFactory(tt.enabled, extras) - factory, err := ProvideFactory(nil, []Extra{extra1}) - require.NoError(t, err) - require.NotNil(t, factory) - }) - - t.Run("should return error when duplicate repository types", func(t *testing.T) { - extra1 := NewMockExtra(t) - extra1.EXPECT().Type().Return(provisioning.GithubConnectionType) - - extra2 := NewMockExtra(t) - extra2.EXPECT().Type().Return(provisioning.GithubConnectionType) - - enabled := map[provisioning.ConnectionType]struct{}{ - provisioning.GithubConnectionType: {}, - } - - factory, err := ProvideFactory(enabled, []Extra{extra1, extra2}) - require.Error(t, err) - assert.Nil(t, factory) - assert.Contains(t, err.Error(), "connection type \"github\" is already registered") - }) + if tt.wantErr { + require.Error(t, err) + assert.Nil(t, factory) + if tt.validateError != nil { + tt.validateError(t, err) + } + } else { + require.NoError(t, err) + require.NotNil(t, factory) + } + }) + } } func TestFactory_Types(t *testing.T) { - t.Run("should return only enabled types that have extras", func(t *testing.T) { - extra1 := NewMockExtra(t) - extra1.EXPECT().Type().Return(provisioning.GithubConnectionType) + tests := []struct { + name string + extraTypes []provisioning.ConnectionType + enabled map[provisioning.ConnectionType]struct{} + expectedLen int + expectedList []provisioning.ConnectionType + checkSorted bool + }{ + { + name: "should return only enabled types that have extras", + extraTypes: []provisioning.ConnectionType{provisioning.GithubConnectionType, provisioning.GitlabConnectionType}, + enabled: map[provisioning.ConnectionType]struct{}{ + provisioning.GithubConnectionType: {}, + provisioning.GitlabConnectionType: {}, + }, + expectedLen: 2, + expectedList: []provisioning.ConnectionType{provisioning.GithubConnectionType, provisioning.GitlabConnectionType}, + }, + { + name: "should return sorted list of types", + extraTypes: []provisioning.ConnectionType{provisioning.GitlabConnectionType, provisioning.GithubConnectionType}, + enabled: map[provisioning.ConnectionType]struct{}{ + provisioning.GithubConnectionType: {}, + provisioning.GitlabConnectionType: {}, + }, + expectedLen: 2, + expectedList: []provisioning.ConnectionType{provisioning.GithubConnectionType, provisioning.GitlabConnectionType}, + checkSorted: true, + }, + { + name: "should return empty list when no types are enabled", + extraTypes: []provisioning.ConnectionType{provisioning.GithubConnectionType}, + enabled: map[provisioning.ConnectionType]struct{}{}, + expectedLen: 0, + expectedList: []provisioning.ConnectionType{}, + }, + { + name: "should not return types that are enabled but have no extras", + extraTypes: []provisioning.ConnectionType{provisioning.GithubConnectionType}, + enabled: map[provisioning.ConnectionType]struct{}{ + provisioning.GithubConnectionType: {}, + provisioning.GitlabConnectionType: {}, + }, + expectedLen: 1, + expectedList: []provisioning.ConnectionType{provisioning.GithubConnectionType}, + }, + { + name: "should not return types that have extras but are not enabled", + extraTypes: []provisioning.ConnectionType{provisioning.GithubConnectionType, provisioning.GitlabConnectionType}, + enabled: map[provisioning.ConnectionType]struct{}{ + provisioning.GithubConnectionType: {}, + }, + expectedLen: 1, + expectedList: []provisioning.ConnectionType{provisioning.GithubConnectionType}, + }, + { + name: "should return empty list when no extras are provided", + extraTypes: []provisioning.ConnectionType{}, + enabled: map[provisioning.ConnectionType]struct{}{ + provisioning.GithubConnectionType: {}, + }, + expectedLen: 0, + expectedList: []provisioning.ConnectionType{}, + }, + } - extra2 := NewMockExtra(t) - extra2.EXPECT().Type().Return(provisioning.GitlabConnectionType) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Setup extras based on the types specified + extras := make([]connection.Extra, 0, len(tt.extraTypes)) + for _, connType := range tt.extraTypes { + extra := connection.NewMockExtra(t) + extra.EXPECT().Type().Return(connType) + extras = append(extras, extra) + } - enabled := map[provisioning.ConnectionType]struct{}{ - provisioning.GithubConnectionType: {}, - provisioning.GitlabConnectionType: {}, - } + factory, err := connection.ProvideFactory(tt.enabled, extras) + require.NoError(t, err) - factory, err := ProvideFactory(enabled, []Extra{extra1, extra2}) - require.NoError(t, err) + types := factory.Types() - types := factory.Types() - assert.Len(t, types, 2) - assert.Contains(t, types, provisioning.GithubConnectionType) - assert.Contains(t, types, provisioning.GitlabConnectionType) - }) + assert.Len(t, types, tt.expectedLen) - t.Run("should return sorted list of types", func(t *testing.T) { - extra1 := NewMockExtra(t) - extra1.EXPECT().Type().Return(provisioning.GitlabConnectionType) - - extra2 := NewMockExtra(t) - extra2.EXPECT().Type().Return(provisioning.GithubConnectionType) - - enabled := map[provisioning.ConnectionType]struct{}{ - provisioning.GithubConnectionType: {}, - provisioning.GitlabConnectionType: {}, - } - - factory, err := ProvideFactory(enabled, []Extra{extra1, extra2}) - require.NoError(t, err) - - types := factory.Types() - assert.Len(t, types, 2) - // github should come before gitlab alphabetically - assert.Equal(t, provisioning.GithubConnectionType, types[0]) - assert.Equal(t, provisioning.GitlabConnectionType, types[1]) - }) - - t.Run("should return empty list when no types are enabled", func(t *testing.T) { - extra1 := NewMockExtra(t) - extra1.EXPECT().Type().Return(provisioning.GithubConnectionType) - - enabled := map[provisioning.ConnectionType]struct{}{} - - factory, err := ProvideFactory(enabled, []Extra{extra1}) - require.NoError(t, err) - - types := factory.Types() - assert.Empty(t, types) - }) - - t.Run("should not return types that are enabled but have no extras", func(t *testing.T) { - extra1 := NewMockExtra(t) - extra1.EXPECT().Type().Return(provisioning.GithubConnectionType) - - enabled := map[provisioning.ConnectionType]struct{}{ - provisioning.GithubConnectionType: {}, - provisioning.GitlabConnectionType: {}, - } - - factory, err := ProvideFactory(enabled, []Extra{extra1}) - require.NoError(t, err) - - types := factory.Types() - assert.Len(t, types, 1) - assert.Contains(t, types, provisioning.GithubConnectionType) - assert.NotContains(t, types, provisioning.GitlabConnectionType) - }) - - t.Run("should not return types that have extras but are not enabled", func(t *testing.T) { - extra1 := NewMockExtra(t) - extra1.EXPECT().Type().Return(provisioning.GithubConnectionType) - - extra2 := NewMockExtra(t) - extra2.EXPECT().Type().Return(provisioning.GitlabConnectionType) - - enabled := map[provisioning.ConnectionType]struct{}{ - provisioning.GithubConnectionType: {}, - } - - factory, err := ProvideFactory(enabled, []Extra{extra1, extra2}) - require.NoError(t, err) - - types := factory.Types() - assert.Len(t, types, 1) - assert.Contains(t, types, provisioning.GithubConnectionType) - assert.NotContains(t, types, provisioning.GitlabConnectionType) - }) - - t.Run("should return empty list when no extras are provided", func(t *testing.T) { - enabled := map[provisioning.ConnectionType]struct{}{ - provisioning.GithubConnectionType: {}, - } - - factory, err := ProvideFactory(enabled, []Extra{}) - require.NoError(t, err) - - types := factory.Types() - assert.Empty(t, types) - }) + if tt.checkSorted { + // Verify exact order: github should come before gitlab alphabetically + assert.Equal(t, tt.expectedList, types) + } else { + // Just verify the types are present + for _, expectedType := range tt.expectedList { + assert.Contains(t, types, expectedType) + } + } + }) + } } func TestFactory_Build(t *testing.T) { - t.Run("should successfully build connection when type is enabled and has extra", func(t *testing.T) { - ctx := context.Background() - conn := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GithubConnectionType, + tests := []struct { + name string + connectionType provisioning.ConnectionType + setupExtras func(t *testing.T, ctx context.Context) ([]connection.Extra, connection.Connection, error) + enabled map[provisioning.ConnectionType]struct{} + wantErr bool + validateError func(t *testing.T, err error) + }{ + { + name: "should successfully build connection when type is enabled and has extra", + connectionType: provisioning.GithubConnectionType, + setupExtras: func(t *testing.T, ctx context.Context) ([]connection.Extra, connection.Connection, error) { + mockConnection := connection.NewMockConnection(t) + extra := connection.NewMockExtra(t) + extra.EXPECT().Type().Return(provisioning.GithubConnectionType) + extra.EXPECT().Build(ctx, &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + }, + }).Return(mockConnection, nil) + + return []connection.Extra{extra}, mockConnection, nil }, - } - - mockConnection := NewMockConnection(t) - extra := NewMockExtra(t) - extra.EXPECT().Type().Return(provisioning.GithubConnectionType) - extra.EXPECT().Build(ctx, conn).Return(mockConnection, nil) - - enabled := map[provisioning.ConnectionType]struct{}{ - provisioning.GithubConnectionType: {}, - } - - factory, err := ProvideFactory(enabled, []Extra{extra}) - require.NoError(t, err) - - result, err := factory.Build(ctx, conn) - require.NoError(t, err) - assert.Equal(t, mockConnection, result) - }) - - t.Run("should return error when type is not enabled", func(t *testing.T) { - ctx := context.Background() - conn := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GitlabConnectionType, + enabled: map[provisioning.ConnectionType]struct{}{ + provisioning.GithubConnectionType: {}, }, - } + wantErr: false, + }, + { + name: "should return error when type is not enabled", + connectionType: provisioning.GitlabConnectionType, + setupExtras: func(t *testing.T, ctx context.Context) ([]connection.Extra, connection.Connection, error) { + extra := connection.NewMockExtra(t) + extra.EXPECT().Type().Return(provisioning.GitlabConnectionType) - extra := NewMockExtra(t) - extra.EXPECT().Type().Return(provisioning.GitlabConnectionType) - - enabled := map[provisioning.ConnectionType]struct{}{ - provisioning.GithubConnectionType: {}, - } - - factory, err := ProvideFactory(enabled, []Extra{extra}) - require.NoError(t, err) - - result, err := factory.Build(ctx, conn) - require.Error(t, err) - assert.Nil(t, result) - assert.Contains(t, err.Error(), "connection type \"gitlab\" is not enabled") - }) - - t.Run("should return error when type is not supported", func(t *testing.T) { - ctx := context.Background() - conn := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GitlabConnectionType, + return []connection.Extra{extra}, nil, nil }, - } - - extra := NewMockExtra(t) - extra.EXPECT().Type().Return(provisioning.GithubConnectionType) - - enabled := map[provisioning.ConnectionType]struct{}{ - provisioning.GithubConnectionType: {}, - } - - factory, err := ProvideFactory(enabled, []Extra{extra}) - require.NoError(t, err) - - result, err := factory.Build(ctx, conn) - require.Error(t, err) - assert.Nil(t, result) - assert.Contains(t, err.Error(), "connection type \"gitlab\" is not supported") - }) - - t.Run("should pass through errors from extra.Build()", func(t *testing.T) { - ctx := context.Background() - conn := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GithubConnectionType, + enabled: map[provisioning.ConnectionType]struct{}{ + provisioning.GithubConnectionType: {}, }, - } - - expectedErr := errors.New("build error") - extra := NewMockExtra(t) - extra.EXPECT().Type().Return(provisioning.GithubConnectionType) - extra.EXPECT().Build(ctx, conn).Return(nil, expectedErr) - - enabled := map[provisioning.ConnectionType]struct{}{ - provisioning.GithubConnectionType: {}, - } - - factory, err := ProvideFactory(enabled, []Extra{extra}) - require.NoError(t, err) - - result, err := factory.Build(ctx, conn) - require.Error(t, err) - assert.Nil(t, result) - assert.Equal(t, expectedErr, err) - }) - - t.Run("should build with multiple extras registered", func(t *testing.T) { - ctx := context.Background() - conn := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GitlabConnectionType, + wantErr: true, + validateError: func(t *testing.T, err error) { + assert.Contains(t, err.Error(), "connection type \"gitlab\" is not enabled") }, - } + }, + { + name: "should return error when type is not supported", + connectionType: provisioning.GitlabConnectionType, + setupExtras: func(t *testing.T, ctx context.Context) ([]connection.Extra, connection.Connection, error) { + extra := connection.NewMockExtra(t) + extra.EXPECT().Type().Return(provisioning.GithubConnectionType) - mockConnection := NewMockConnection(t) + return []connection.Extra{extra}, nil, nil + }, + enabled: map[provisioning.ConnectionType]struct{}{ + provisioning.GithubConnectionType: {}, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + assert.Contains(t, err.Error(), "connection type \"gitlab\" is not supported") + }, + }, + { + name: "should pass through errors from extra.Build()", + connectionType: provisioning.GithubConnectionType, + setupExtras: func(t *testing.T, ctx context.Context) ([]connection.Extra, connection.Connection, error) { + buildErr := errors.New("build error") + extra := connection.NewMockExtra(t) + extra.EXPECT().Type().Return(provisioning.GithubConnectionType) + extra.EXPECT().Build(ctx, &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + }, + }).Return(nil, buildErr) - extra1 := NewMockExtra(t) - extra1.EXPECT().Type().Return(provisioning.GithubConnectionType) + return []connection.Extra{extra}, nil, buildErr + }, + enabled: map[provisioning.ConnectionType]struct{}{ + provisioning.GithubConnectionType: {}, + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + assert.Equal(t, "build error", err.Error()) + }, + }, + { + name: "should build with multiple extras registered", + connectionType: provisioning.GitlabConnectionType, + setupExtras: func(t *testing.T, ctx context.Context) ([]connection.Extra, connection.Connection, error) { + mockConnection := connection.NewMockConnection(t) - extra2 := NewMockExtra(t) - extra2.EXPECT().Type().Return(provisioning.GitlabConnectionType) - extra2.EXPECT().Build(ctx, conn).Return(mockConnection, nil) + extra1 := connection.NewMockExtra(t) + extra1.EXPECT().Type().Return(provisioning.GithubConnectionType) - enabled := map[provisioning.ConnectionType]struct{}{ - provisioning.GithubConnectionType: {}, - provisioning.GitlabConnectionType: {}, - } + extra2 := connection.NewMockExtra(t) + extra2.EXPECT().Type().Return(provisioning.GitlabConnectionType) + extra2.EXPECT().Build(ctx, &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GitlabConnectionType, + }, + }).Return(mockConnection, nil) - factory, err := ProvideFactory(enabled, []Extra{extra1, extra2}) - require.NoError(t, err) + return []connection.Extra{extra1, extra2}, mockConnection, nil + }, + enabled: map[provisioning.ConnectionType]struct{}{ + provisioning.GithubConnectionType: {}, + provisioning.GitlabConnectionType: {}, + }, + wantErr: false, + }, + } - result, err := factory.Build(ctx, conn) - require.NoError(t, err) - assert.Equal(t, mockConnection, result) - }) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + extras, expectedConnection, _ := tt.setupExtras(t, ctx) + + factory, err := connection.ProvideFactory(tt.enabled, extras) + require.NoError(t, err) + + conn := &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: tt.connectionType, + }, + } + + result, err := factory.Build(ctx, conn) + + if tt.wantErr { + require.Error(t, err) + assert.Nil(t, result) + if tt.validateError != nil { + tt.validateError(t, err) + } + } else { + require.NoError(t, err) + assert.Equal(t, expectedConnection, result) + } + }) + } } diff --git a/apps/provisioning/pkg/connection/github/client.go b/apps/provisioning/pkg/connection/github/client.go index 7ddb9a4665e..b5eaa173217 100644 --- a/apps/provisioning/pkg/connection/github/client.go +++ b/apps/provisioning/pkg/connection/github/client.go @@ -22,6 +22,7 @@ type Client interface { // Apps and installations GetApp(ctx context.Context) (App, error) GetAppInstallation(ctx context.Context, installationID string) (AppInstallation, error) + CreateInstallationAccessToken(ctx context.Context, installationID string, repo string) (InstallationToken, error) } // App represents a Github App. @@ -42,6 +43,14 @@ type AppInstallation struct { Enabled bool } +// InstallationToken represents a Github App Installation Access Token. +type InstallationToken struct { + // Token is the access token value. + Token string + // ExpiresAt is the expiration time of the token. + ExpiresAt string +} + type githubClient struct { gh *github.Client } @@ -61,7 +70,6 @@ func (r *githubClient) GetApp(ctx context.Context) (App, error) { return App{}, err } - // TODO(ferruvich): do we need any other info? return App{ ID: app.GetID(), Slug: app.GetSlug(), @@ -85,9 +93,34 @@ func (r *githubClient) GetAppInstallation(ctx context.Context, installationID st return AppInstallation{}, err } - // TODO(ferruvich): do we need any other info? return AppInstallation{ ID: installation.GetID(), Enabled: installation.GetSuspendedAt().IsZero(), }, nil } + +// CreateInstallationAccessToken creates an installation access token scoped to a specific repository. +func (r *githubClient) CreateInstallationAccessToken(ctx context.Context, installationID string, repo string) (InstallationToken, error) { + id, err := strconv.Atoi(installationID) + if err != nil { + return InstallationToken{}, fmt.Errorf("invalid installation ID: %s", installationID) + } + + opts := &github.InstallationTokenOptions{ + Repositories: []string{repo}, + } + + token, _, err := r.gh.Apps.CreateInstallationToken(ctx, int64(id), opts) + if err != nil { + var ghErr *github.ErrorResponse + if errors.As(err, &ghErr) && ghErr.Response.StatusCode == http.StatusServiceUnavailable { + return InstallationToken{}, ErrServiceUnavailable + } + return InstallationToken{}, err + } + + return InstallationToken{ + Token: token.GetToken(), + ExpiresAt: token.GetExpiresAt().String(), + }, nil +} diff --git a/apps/provisioning/pkg/connection/github/client_mock.go b/apps/provisioning/pkg/connection/github/client_mock.go index c9f009f5021..005e5ed1b1c 100644 --- a/apps/provisioning/pkg/connection/github/client_mock.go +++ b/apps/provisioning/pkg/connection/github/client_mock.go @@ -21,6 +21,64 @@ func (_m *MockClient) EXPECT() *MockClient_Expecter { return &MockClient_Expecter{mock: &_m.Mock} } +// CreateInstallationAccessToken provides a mock function with given fields: ctx, installationID, repo +func (_m *MockClient) CreateInstallationAccessToken(ctx context.Context, installationID string, repo string) (InstallationToken, error) { + ret := _m.Called(ctx, installationID, repo) + + if len(ret) == 0 { + panic("no return value specified for CreateInstallationAccessToken") + } + + var r0 InstallationToken + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string, string) (InstallationToken, error)); ok { + return rf(ctx, installationID, repo) + } + if rf, ok := ret.Get(0).(func(context.Context, string, string) InstallationToken); ok { + r0 = rf(ctx, installationID, repo) + } else { + r0 = ret.Get(0).(InstallationToken) + } + + if rf, ok := ret.Get(1).(func(context.Context, string, string) error); ok { + r1 = rf(ctx, installationID, repo) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockClient_CreateInstallationAccessToken_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CreateInstallationAccessToken' +type MockClient_CreateInstallationAccessToken_Call struct { + *mock.Call +} + +// CreateInstallationAccessToken is a helper method to define mock.On call +// - ctx context.Context +// - installationID string +// - repo string +func (_e *MockClient_Expecter) CreateInstallationAccessToken(ctx interface{}, installationID interface{}, repo interface{}) *MockClient_CreateInstallationAccessToken_Call { + return &MockClient_CreateInstallationAccessToken_Call{Call: _e.mock.On("CreateInstallationAccessToken", ctx, installationID, repo)} +} + +func (_c *MockClient_CreateInstallationAccessToken_Call) Run(run func(ctx context.Context, installationID string, repo string)) *MockClient_CreateInstallationAccessToken_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(string)) + }) + return _c +} + +func (_c *MockClient_CreateInstallationAccessToken_Call) Return(_a0 InstallationToken, _a1 error) *MockClient_CreateInstallationAccessToken_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockClient_CreateInstallationAccessToken_Call) RunAndReturn(run func(context.Context, string, string) (InstallationToken, error)) *MockClient_CreateInstallationAccessToken_Call { + _c.Call.Return(run) + return _c +} + // GetApp provides a mock function with given fields: ctx func (_m *MockClient) GetApp(ctx context.Context) (App, error) { ret := _m.Called(ctx) diff --git a/apps/provisioning/pkg/connection/github/client_test.go b/apps/provisioning/pkg/connection/github/client_test.go index bae6d6ac1e9..f6a53d50969 100644 --- a/apps/provisioning/pkg/connection/github/client_test.go +++ b/apps/provisioning/pkg/connection/github/client_test.go @@ -295,3 +295,175 @@ func TestGithubClient_GetAppInstallation(t *testing.T) { }) } } + +func TestGithubClient_CreateInstallationAccessToken(t *testing.T) { + tests := []struct { + name string + mockHandler *http.Client + installationID string + repo string + wantToken conngh.InstallationToken + wantErr bool + errContains string + }{ + { + name: "create installation token successfully", + mockHandler: mockhub.NewMockedHTTPClient( + mockhub.WithRequestMatchHandler( + mockhub.PostAppInstallationsAccessTokensByInstallationId, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + expiresAt := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC) + token := &github.InstallationToken{ + Token: github.Ptr("ghs_test_token_123456789"), + ExpiresAt: &github.Timestamp{Time: expiresAt}, + } + w.WriteHeader(http.StatusCreated) + require.NoError(t, json.NewEncoder(w).Encode(token)) + }), + ), + ), + installationID: "12345", + repo: "test-repo", + wantToken: conngh.InstallationToken{ + Token: "ghs_test_token_123456789", + ExpiresAt: "2024-01-01 00:00:00 +0000 UTC", + }, + wantErr: false, + }, + { + name: "invalid installation ID", + mockHandler: mockhub.NewMockedHTTPClient(), + installationID: "not-a-number", + repo: "test-repo", + wantToken: conngh.InstallationToken{}, + wantErr: true, + errContains: "invalid installation ID", + }, + { + name: "service unavailable", + mockHandler: mockhub.NewMockedHTTPClient( + mockhub.WithRequestMatchHandler( + mockhub.PostAppInstallationsAccessTokensByInstallationId, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusServiceUnavailable) + require.NoError(t, json.NewEncoder(w).Encode(github.ErrorResponse{ + Response: &http.Response{ + StatusCode: http.StatusServiceUnavailable, + }, + Message: "Service unavailable", + })) + }), + ), + ), + installationID: "12345", + repo: "test-repo", + wantToken: conngh.InstallationToken{}, + wantErr: true, + errContains: conngh.ErrServiceUnavailable.Error(), + }, + { + name: "installation not found", + mockHandler: mockhub.NewMockedHTTPClient( + mockhub.WithRequestMatchHandler( + mockhub.PostAppInstallationsAccessTokensByInstallationId, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + require.NoError(t, json.NewEncoder(w).Encode(github.ErrorResponse{ + Response: &http.Response{ + StatusCode: http.StatusNotFound, + }, + Message: "Not Found", + })) + }), + ), + ), + installationID: "99999", + repo: "test-repo", + wantToken: conngh.InstallationToken{}, + wantErr: true, + }, + { + name: "unauthorized error", + mockHandler: mockhub.NewMockedHTTPClient( + mockhub.WithRequestMatchHandler( + mockhub.PostAppInstallationsAccessTokensByInstallationId, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + require.NoError(t, json.NewEncoder(w).Encode(github.ErrorResponse{ + Response: &http.Response{ + StatusCode: http.StatusUnauthorized, + }, + Message: "Bad credentials", + })) + }), + ), + ), + installationID: "12345", + repo: "test-repo", + wantToken: conngh.InstallationToken{}, + wantErr: true, + }, + { + name: "forbidden - no permissions for repository", + mockHandler: mockhub.NewMockedHTTPClient( + mockhub.WithRequestMatchHandler( + mockhub.PostAppInstallationsAccessTokensByInstallationId, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + require.NoError(t, json.NewEncoder(w).Encode(github.ErrorResponse{ + Response: &http.Response{ + StatusCode: http.StatusForbidden, + }, + Message: "Resource not accessible by integration", + })) + }), + ), + ), + installationID: "12345", + repo: "private-repo", + wantToken: conngh.InstallationToken{}, + wantErr: true, + }, + { + name: "internal server error", + mockHandler: mockhub.NewMockedHTTPClient( + mockhub.WithRequestMatchHandler( + mockhub.PostAppInstallationsAccessTokensByInstallationId, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + require.NoError(t, json.NewEncoder(w).Encode(github.ErrorResponse{ + Response: &http.Response{ + StatusCode: http.StatusInternalServerError, + }, + Message: "Internal server error", + })) + }), + ), + ), + installationID: "12345", + repo: "test-repo", + wantToken: conngh.InstallationToken{}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ghClient := github.NewClient(tt.mockHandler) + client := conngh.NewClient(ghClient) + + token, err := client.CreateInstallationAccessToken(context.Background(), tt.installationID, tt.repo) + + if tt.wantErr { + assert.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + } else { + assert.NoError(t, err) + } + + assert.Equal(t, tt.wantToken, token) + }) + } +} diff --git a/apps/provisioning/pkg/connection/github/connection.go b/apps/provisioning/pkg/connection/github/connection.go index 6a2da98ac8d..53a465bea2b 100644 --- a/apps/provisioning/pkg/connection/github/connection.go +++ b/apps/provisioning/pkg/connection/github/connection.go @@ -10,6 +10,7 @@ import ( "github.com/golang-jwt/jwt/v4" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/apps/provisioning/pkg/connection" + "github.com/grafana/grafana/apps/provisioning/pkg/repository/github" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/validation/field" @@ -20,18 +21,26 @@ type GithubFactory interface { New(ctx context.Context, ghToken common.RawSecureValue) Client } +type ConnectionSecrets struct { + PrivateKey common.RawSecureValue + Token common.RawSecureValue +} + type Connection struct { obj *provisioning.Connection ghFactory GithubFactory + secrets ConnectionSecrets } func NewConnection( obj *provisioning.Connection, factory GithubFactory, + secrets ConnectionSecrets, ) Connection { return Connection{ obj: obj, ghFactory: factory, + secrets: secrets, } } @@ -51,14 +60,15 @@ func (c *Connection) Mutate(_ context.Context) error { c.obj.Spec.URL = fmt.Sprintf("%s/%s", githubInstallationURL, c.obj.Spec.GitHub.InstallationID) - // Generate JWT token if private key is being provided. - // Same as for the spec.Github, if such a field is required, Validation will take care of that. - if !c.obj.Secure.PrivateKey.Create.IsZero() { - token, err := generateToken(c.obj.Spec.GitHub.AppID, c.obj.Secure.PrivateKey.Create) + // Generate token only if one of the following cases are true + // - The object is being created now (generation == 0) + // - The token is not there + // - A new Private key is being submitted + if c.obj.Generation == 0 || c.secrets.Token.IsZero() || !c.obj.Secure.PrivateKey.Create.IsZero() { + token, err := generateToken(c.obj.Spec.GitHub.AppID, c.secrets.PrivateKey) if err != nil { return fmt.Errorf("failed to generate JWT token: %w", err) } - // Store the generated token c.obj.Secure.Token = common.InlineSecureValue{Create: token} } @@ -117,10 +127,10 @@ func (c *Connection) Validate(ctx context.Context) error { return toError(c.obj.GetName(), list) } - if c.obj.Secure.PrivateKey.IsZero() { + if c.secrets.PrivateKey.IsZero() { list = append(list, field.Required(field.NewPath("secure", "privateKey"), "privateKey must be specified for GitHub connection")) } - if c.obj.Secure.Token.IsZero() { + if c.secrets.Token.IsZero() { list = append(list, field.Required(field.NewPath("secure", "token"), "token must be specified for GitHub connection")) } if !c.obj.Secure.ClientSecret.IsZero() { @@ -150,7 +160,7 @@ func (c *Connection) Validate(ctx context.Context) error { // validateAppAndInstallation validates the appID and installationID against the given github token. func (c *Connection) validateAppAndInstallation(ctx context.Context) *field.Error { - ghClient := c.ghFactory.New(ctx, c.obj.Secure.Token.Create) + ghClient := c.ghFactory.New(ctx, c.secrets.Token) app, err := ghClient.GetApp(ctx) if err != nil { @@ -187,6 +197,35 @@ func toError(name string, list field.ErrorList) error { ) } +// GenerateRepositoryToken generates a repository-scoped access token. +func (c *Connection) GenerateRepositoryToken(ctx context.Context, repo *provisioning.Repository) (common.RawSecureValue, error) { + if repo == nil { + return "", errors.New("a repository is required to generate a token") + } + if c.obj.Spec.GitHub == nil { + return "", errors.New("connection is not a GitHub connection") + } + if repo.Spec.GitHub == nil { + return "", errors.New("repository is not a GitHub repo") + } + + _, repoName, err := github.ParseOwnerRepoGithub(repo.Spec.GitHub.URL) + if err != nil { + return "", fmt.Errorf("failed to parse repo URL: %w", err) + } + + // Create the GitHub client with the JWT token + ghClient := c.ghFactory.New(ctx, c.secrets.Token) + + // Create an installation access token scoped to this repository + installationToken, err := ghClient.CreateInstallationAccessToken(ctx, c.obj.Spec.GitHub.InstallationID, repoName) + if err != nil { + return "", fmt.Errorf("failed to create installation access token: %w", err) + } + + return common.RawSecureValue(installationToken.Token), nil +} + var ( _ connection.Connection = (*Connection)(nil) ) diff --git a/apps/provisioning/pkg/connection/github/connection_test.go b/apps/provisioning/pkg/connection/github/connection_test.go index 6a916db730e..bd8794edae3 100644 --- a/apps/provisioning/pkg/connection/github/connection_test.go +++ b/apps/provisioning/pkg/connection/github/connection_test.go @@ -1,11 +1,13 @@ -package github +package github_test import ( "context" "encoding/base64" + "errors" "testing" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/apps/provisioning/pkg/connection/github" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -43,132 +45,386 @@ B8Uc0WUgheB4+yVKGnYpYaSOgFFI5+1BYUva/wDHLy2pWHz39Usb -----END RSA PRIVATE KEY-----` func TestConnection_Mutate(t *testing.T) { - t.Run("should add URL to Github connection", func(t *testing.T) { - c := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GithubConnectionType, - GitHub: &provisioning.GitHubConnectionConfig{ - AppID: "123", - InstallationID: "456", + privateKeyBase64 := base64.StdEncoding.EncodeToString([]byte(testPrivateKeyPEM)) + + tests := []struct { + name string + connection *provisioning.Connection + secrets github.ConnectionSecrets + wantErr bool + validateError func(t *testing.T, err error) + validateResult func(t *testing.T, connection *provisioning.Connection) + }{ + { + name: "should add URL to Github connection", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123", + InstallationID: "456", + }, + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{ + Create: common.NewSecretValue(privateKeyBase64), + }, }, }, - Secure: provisioning.ConnectionSecure{ - PrivateKey: common.InlineSecureValue{ - Name: "test-private-key", + secrets: github.ConnectionSecrets{ + PrivateKey: common.NewSecretValue(privateKeyBase64), + }, + wantErr: false, + validateResult: func(t *testing.T, connection *provisioning.Connection) { + assert.Equal(t, "https://github.com/settings/installations/456", connection.Spec.URL) + }, + }, + { + name: "should generate JWT token when private key is provided", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123", + InstallationID: "456", + }, + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{ + Create: common.NewSecretValue(privateKeyBase64), + }, }, }, - } - - mockFactory := NewMockGithubFactory(t) - conn := NewConnection(c, mockFactory) - - require.NoError(t, conn.Mutate(context.Background())) - assert.Equal(t, "https://github.com/settings/installations/456", c.Spec.URL) - }) - - t.Run("should generate JWT token when private key is provided", func(t *testing.T) { - privateKeyBase64 := base64.StdEncoding.EncodeToString([]byte(testPrivateKeyPEM)) - - c := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GithubConnectionType, - GitHub: &provisioning.GitHubConnectionConfig{ - AppID: "123", - InstallationID: "456", + secrets: github.ConnectionSecrets{ + PrivateKey: common.NewSecretValue(privateKeyBase64), + }, + wantErr: false, + validateResult: func(t *testing.T, connection *provisioning.Connection) { + assert.Equal(t, "https://github.com/settings/installations/456", connection.Spec.URL) + assert.False(t, connection.Secure.Token.Create.IsZero(), "JWT token should be generated") + }, + }, + { + name: "should not generate JWT token when token is already there", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection", Generation: 1}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123", + InstallationID: "456", + }, + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{ + // The private key is already in the stoere + Name: "somePrivateKey", + }, + Token: common.InlineSecureValue{ + Create: common.NewSecretValue("someToken"), + }, }, }, - Secure: provisioning.ConnectionSecure{ - PrivateKey: common.InlineSecureValue{ - Create: common.NewSecretValue(privateKeyBase64), + secrets: github.ConnectionSecrets{ + PrivateKey: common.NewSecretValue(privateKeyBase64), + Token: common.NewSecretValue("someToken"), + }, + wantErr: false, + validateResult: func(t *testing.T, connection *provisioning.Connection) { + assert.Equal(t, "https://github.com/settings/installations/456", connection.Spec.URL) + assert.False(t, connection.Secure.Token.Create.IsZero(), "JWT token should be generated") + assert.Equal(t, "someToken", connection.Secure.Token.Create.DangerouslyExposeAndConsumeValue()) + }, + }, + { + name: "should do nothing when GitHub config is nil", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GitlabConnectionType, + Gitlab: &provisioning.GitlabConnectionConfig{ + ClientID: "clientID", + }, }, }, - } - - mockFactory := NewMockGithubFactory(t) - conn := NewConnection(c, mockFactory) - - require.NoError(t, conn.Mutate(context.Background())) - assert.Equal(t, "https://github.com/settings/installations/456", c.Spec.URL) - assert.False(t, c.Secure.Token.Create.IsZero(), "JWT token should be generated") - }) - - t.Run("should do nothing when GitHub config is nil", func(t *testing.T) { - c := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GitlabConnectionType, - Gitlab: &provisioning.GitlabConnectionConfig{ - ClientID: "clientID", + secrets: github.ConnectionSecrets{}, + wantErr: false, + }, + { + name: "should fail when private key is not base64", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123", + InstallationID: "456", + }, + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{ + Create: common.NewSecretValue("invalid-key"), + }, }, }, - } - - mockFactory := NewMockGithubFactory(t) - conn := NewConnection(c, mockFactory) - - require.NoError(t, conn.Mutate(context.Background())) - }) - - t.Run("should fail when private key is not base64", func(t *testing.T) { - c := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GithubConnectionType, - GitHub: &provisioning.GitHubConnectionConfig{ - AppID: "123", - InstallationID: "456", + secrets: github.ConnectionSecrets{ + PrivateKey: "invalid-key", + }, + wantErr: true, + validateError: func(t *testing.T, err error) { + assert.Contains(t, err.Error(), "failed to generate JWT token") + assert.Contains(t, err.Error(), "failed to decode base64 private key") + }, + }, + { + name: "should fail when private key is invalid", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123", + InstallationID: "456", + }, + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{ + Create: common.NewSecretValue(base64.StdEncoding.EncodeToString([]byte("invalid-key"))), + }, }, }, - Secure: provisioning.ConnectionSecure{ - PrivateKey: common.InlineSecureValue{ - Create: common.NewSecretValue("invalid-key"), + secrets: github.ConnectionSecrets{}, + wantErr: true, + validateError: func(t *testing.T, err error) { + assert.Contains(t, err.Error(), "failed to generate JWT token") + assert.Contains(t, err.Error(), "failed to parse private key") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockFactory := github.NewMockGithubFactory(t) + conn := github.NewConnection(tt.connection, mockFactory, tt.secrets) + + err := conn.Mutate(context.Background()) + + if tt.wantErr { + require.Error(t, err) + if tt.validateError != nil { + tt.validateError(t, err) + } + } else { + require.NoError(t, err) + if tt.validateResult != nil { + tt.validateResult(t, tt.connection) + } + } + }) + } +} + +func TestConnection_GenerateRepositoryToken(t *testing.T) { + tests := []struct { + name string + connection *provisioning.Connection + repo *provisioning.Repository + setupMock func(*github.MockGithubFactory) + expectedToken common.RawSecureValue + expectedError string + }{ + { + name: "success", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123", + InstallationID: "456", + }, + }, + Secure: provisioning.ConnectionSecure{ + Token: common.InlineSecureValue{ + Create: common.RawSecureValue("jwt-token"), + }, }, }, - } - - mockFactory := NewMockGithubFactory(t) - conn := NewConnection(c, mockFactory) - - err := conn.Mutate(context.Background()) - require.Error(t, err) - assert.Contains(t, err.Error(), "failed to generate JWT token") - assert.Contains(t, err.Error(), "failed to decode base64 private key") - }) - - t.Run("should fail when private key is invalid", func(t *testing.T) { - c := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GithubConnectionType, - GitHub: &provisioning.GitHubConnectionConfig{ - AppID: "123", - InstallationID: "456", + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{Name: "test-repo"}, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test-owner/test-repo", + }, }, }, - Secure: provisioning.ConnectionSecure{ - PrivateKey: common.InlineSecureValue{ - Create: common.NewSecretValue(base64.StdEncoding.EncodeToString([]byte("invalid-key"))), + setupMock: func(mockFactory *github.MockGithubFactory) { + mockClient := github.NewMockClient(t) + mockFactory.EXPECT().New(mock.Anything, common.RawSecureValue("jwt-token")).Return(mockClient) + mockClient.EXPECT().CreateInstallationAccessToken(mock.Anything, "456", "test-repo"). + Return(github.InstallationToken{Token: "ghs_repository_token_123", ExpiresAt: "2024-01-01T00:00:00Z"}, nil) + }, + expectedToken: common.RawSecureValue("ghs_repository_token_123"), + }, + { + name: "nil repository returns error", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123", + InstallationID: "456", + }, }, }, - } + repo: nil, + expectedError: "a repository is required to generate a token", + }, + { + name: "connection without GitHub config returns error", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GitlabConnectionType, + Gitlab: &provisioning.GitlabConnectionConfig{ + ClientID: "clientID", + }, + }, + }, + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{Name: "test-repo"}, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test-owner/test-repo", + }, + }, + }, + expectedError: "connection is not a GitHub connection", + }, + { + name: "repository without GitHub config returns error", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123", + InstallationID: "456", + }, + }, + Secure: provisioning.ConnectionSecure{ + Token: common.InlineSecureValue{ + Create: common.RawSecureValue("jwt-token"), + }, + }, + }, + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{Name: "test-repo"}, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: nil, + }, + }, + expectedError: "repository is not a GitHub repo", + }, + { + name: "invalid repository URL returns error", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123", + InstallationID: "456", + }, + }, + Secure: provisioning.ConnectionSecure{ + Token: common.InlineSecureValue{ + Create: common.RawSecureValue("jwt-token"), + }, + }, + }, + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{Name: "test-repo"}, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "invalid-url", + }, + }, + }, + expectedError: "failed to parse repo URL", + }, + { + name: "GitHub API error", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123", + InstallationID: "456", + }, + }, + Secure: provisioning.ConnectionSecure{ + Token: common.InlineSecureValue{ + Create: common.RawSecureValue("jwt-token"), + }, + }, + }, + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{Name: "test-repo"}, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test-owner/test-repo", + }, + }, + }, + setupMock: func(mockFactory *github.MockGithubFactory) { + mockClient := github.NewMockClient(t) + mockFactory.EXPECT().New(mock.Anything, common.RawSecureValue("jwt-token")).Return(mockClient) + mockClient.EXPECT().CreateInstallationAccessToken(mock.Anything, "456", "test-repo"). + Return(github.InstallationToken{}, errors.New("API rate limit exceeded")) + }, + expectedError: "failed to create installation access token", + }, + } - mockFactory := NewMockGithubFactory(t) - conn := NewConnection(c, mockFactory) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockFactory := github.NewMockGithubFactory(t) + if tt.setupMock != nil { + tt.setupMock(mockFactory) + } - err := conn.Mutate(context.Background()) - require.Error(t, err) - assert.Contains(t, err.Error(), "failed to generate JWT token") - assert.Contains(t, err.Error(), "failed to parse private key") - }) + conn := github.NewConnection(tt.connection, mockFactory, github.ConnectionSecrets{ + Token: tt.connection.Secure.Token.Create, + PrivateKey: tt.connection.Secure.PrivateKey.Create, + }) + token, err := conn.GenerateRepositoryToken(context.Background(), tt.repo) + + if tt.expectedError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedError) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expectedToken, token) + } + }) + } } func TestConnection_Validate(t *testing.T) { tests := []struct { name string connection *provisioning.Connection - setupMock func(*MockGithubFactory) + setupMock func(*github.MockGithubFactory) wantErr bool errMsgContains []string }{ @@ -314,12 +570,12 @@ func TestConnection_Validate(t *testing.T) { }, }, wantErr: false, - setupMock: func(mockFactory *MockGithubFactory) { - mockClient := NewMockClient(t) + setupMock: func(mockFactory *github.MockGithubFactory) { + mockClient := github.NewMockClient(t) mockFactory.EXPECT().New(mock.Anything, common.RawSecureValue("test-token")).Return(mockClient) - mockClient.EXPECT().GetApp(mock.Anything).Return(App{ID: 123, Slug: "test-app"}, nil) - mockClient.EXPECT().GetAppInstallation(mock.Anything, "456").Return(AppInstallation{ID: 456}, nil) + mockClient.EXPECT().GetApp(mock.Anything).Return(github.App{ID: 123, Slug: "test-app"}, nil) + mockClient.EXPECT().GetAppInstallation(mock.Anything, "456").Return(github.AppInstallation{ID: 456}, nil) }, }, { @@ -344,11 +600,11 @@ func TestConnection_Validate(t *testing.T) { }, wantErr: true, errMsgContains: []string{"spec.token", "[REDACTED]"}, - setupMock: func(mockFactory *MockGithubFactory) { - mockClient := NewMockClient(t) + setupMock: func(mockFactory *github.MockGithubFactory) { + mockClient := github.NewMockClient(t) mockFactory.EXPECT().New(mock.Anything, common.RawSecureValue("test-token")).Return(mockClient) - mockClient.EXPECT().GetApp(mock.Anything).Return(App{}, assert.AnError) + mockClient.EXPECT().GetApp(mock.Anything).Return(github.App{}, assert.AnError) }, }, { @@ -373,11 +629,11 @@ func TestConnection_Validate(t *testing.T) { }, wantErr: true, errMsgContains: []string{"spec.appID"}, - setupMock: func(mockFactory *MockGithubFactory) { - mockClient := NewMockClient(t) + setupMock: func(mockFactory *github.MockGithubFactory) { + mockClient := github.NewMockClient(t) mockFactory.EXPECT().New(mock.Anything, common.RawSecureValue("test-token")).Return(mockClient) - mockClient.EXPECT().GetApp(mock.Anything).Return(App{ID: 444, Slug: "test-app"}, nil) + mockClient.EXPECT().GetApp(mock.Anything).Return(github.App{ID: 444, Slug: "test-app"}, nil) }, }, { @@ -402,24 +658,27 @@ func TestConnection_Validate(t *testing.T) { }, wantErr: true, errMsgContains: []string{"spec.installationID", "456"}, - setupMock: func(mockFactory *MockGithubFactory) { - mockClient := NewMockClient(t) + setupMock: func(mockFactory *github.MockGithubFactory) { + mockClient := github.NewMockClient(t) mockFactory.EXPECT().New(mock.Anything, common.RawSecureValue("test-token")).Return(mockClient) - mockClient.EXPECT().GetApp(mock.Anything).Return(App{ID: 123, Slug: "test-app"}, nil) - mockClient.EXPECT().GetAppInstallation(mock.Anything, "456").Return(AppInstallation{}, assert.AnError) + mockClient.EXPECT().GetApp(mock.Anything).Return(github.App{ID: 123, Slug: "test-app"}, nil) + mockClient.EXPECT().GetAppInstallation(mock.Anything, "456").Return(github.AppInstallation{}, assert.AnError) }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mockFactory := NewMockGithubFactory(t) + mockFactory := github.NewMockGithubFactory(t) if tt.setupMock != nil { tt.setupMock(mockFactory) } - conn := NewConnection(tt.connection, mockFactory) + conn := github.NewConnection(tt.connection, mockFactory, github.ConnectionSecrets{ + PrivateKey: tt.connection.Secure.PrivateKey.Create, + Token: tt.connection.Secure.Token.Create, + }) err := conn.Validate(context.Background()) if tt.wantErr { assert.Error(t, err) diff --git a/apps/provisioning/pkg/connection/github/extra.go b/apps/provisioning/pkg/connection/github/extra.go index 2c207637c61..b23b3d10884 100644 --- a/apps/provisioning/pkg/connection/github/extra.go +++ b/apps/provisioning/pkg/connection/github/extra.go @@ -10,27 +10,51 @@ import ( ) type extra struct { - factory GithubFactory + factory GithubFactory + decrypter connection.Decrypter } func (e *extra) Type() provisioning.ConnectionType { return provisioning.GithubConnectionType } -func (e *extra) Build(ctx context.Context, connection *provisioning.Connection) (connection.Connection, error) { +func (e *extra) Build(ctx context.Context, conn *provisioning.Connection) (connection.Connection, error) { logger := logging.FromContext(ctx) - if connection == nil || connection.Spec.GitHub == nil { + if conn == nil || conn.Spec.GitHub == nil { logger.Error("connection is nil or github info is nil") return nil, fmt.Errorf("invalid github connection") } - c := NewConnection(connection, e.factory) + // Decrypt secure values + secure := e.decrypter(conn) + + // Decrypt private key + pKey, err := secure.PrivateKey(ctx) + if err != nil { + logger.Error("Failed to decrypt private key", "error", err) + + return nil, err + } + + // Decrypt token + t, err := secure.Token(ctx) + if err != nil { + logger.Error("Failed to decrypt token", "error", err) + + return nil, err + } + + c := NewConnection(conn, e.factory, ConnectionSecrets{ + PrivateKey: pKey, + Token: t, + }) return &c, nil } -func Extra(factory GithubFactory) connection.Extra { +func Extra(decrypter connection.Decrypter, factory GithubFactory) connection.Extra { return &extra{ - factory: factory, + decrypter: decrypter, + factory: factory, } } diff --git a/apps/provisioning/pkg/connection/github/extra_test.go b/apps/provisioning/pkg/connection/github/extra_test.go index c5bcc8279d9..d87ca9629ea 100644 --- a/apps/provisioning/pkg/connection/github/extra_test.go +++ b/apps/provisioning/pkg/connection/github/extra_test.go @@ -2,9 +2,11 @@ package github_test import ( "context" + "errors" "testing" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/apps/provisioning/pkg/connection" "github.com/grafana/grafana/apps/provisioning/pkg/connection/github" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" "github.com/stretchr/testify/assert" @@ -12,115 +14,218 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +type mockSecureValues struct { + privateKey common.RawSecureValue + privateKeyErr error + clientSecret common.RawSecureValue + clientSecErr error + token common.RawSecureValue + tokenErr error +} + +func (m *mockSecureValues) PrivateKey(_ context.Context) (common.RawSecureValue, error) { + return m.privateKey, m.privateKeyErr +} + +func (m *mockSecureValues) ClientSecret(_ context.Context) (common.RawSecureValue, error) { + return m.clientSecret, m.clientSecErr +} + +func (m *mockSecureValues) Token(_ context.Context) (common.RawSecureValue, error) { + return m.token, m.tokenErr +} + func TestExtra_Type(t *testing.T) { - t.Run("should return GithubConnectionType", func(t *testing.T) { - mockFactory := github.NewMockGithubFactory(t) - e := github.Extra(mockFactory) - result := e.Type() - assert.Equal(t, provisioning.GithubConnectionType, result) - }) + mockFactory := github.NewMockGithubFactory(t) + decrypter := func(c *provisioning.Connection) connection.SecureValues { + return &mockSecureValues{} + } + + e := github.Extra(decrypter, mockFactory) + + result := e.Type() + + assert.Equal(t, provisioning.GithubConnectionType, result) } func TestExtra_Build(t *testing.T) { - t.Run("should successfully build connection", func(t *testing.T) { - ctx := context.Background() - conn := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GithubConnectionType, - GitHub: &provisioning.GitHubConnectionConfig{ - AppID: "123", - InstallationID: "456", + tests := []struct { + name string + conn *provisioning.Connection + setupDecrypter func() connection.Decrypter + expectedError string + }{ + { + name: "success with valid connection", + conn: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123456", + InstallationID: "789012", + }, }, }, - Secure: provisioning.ConnectionSecure{ - PrivateKey: common.InlineSecureValue{ - Create: common.NewSecretValue("test-private-key"), + setupDecrypter: func() connection.Decrypter { + return func(c *provisioning.Connection) connection.SecureValues { + return &mockSecureValues{ + privateKey: common.RawSecureValue("test-private-key"), + token: common.RawSecureValue("test-token"), + } + } + }, + }, + { + name: "nil connection", + conn: nil, + setupDecrypter: func() connection.Decrypter { + return func(c *provisioning.Connection) connection.SecureValues { + return &mockSecureValues{} + } + }, + expectedError: "invalid github connection", + }, + { + name: "connection without github config", + conn: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: nil, }, }, - } - - mockFactory := github.NewMockGithubFactory(t) - - e := github.Extra(mockFactory) - - result, err := e.Build(ctx, conn) - require.NoError(t, err) - require.NotNil(t, result) - }) - - t.Run("should handle different connection configurations", func(t *testing.T) { - ctx := context.Background() - conn := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "another-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GithubConnectionType, - GitHub: &provisioning.GitHubConnectionConfig{ - AppID: "789", - InstallationID: "101112", + setupDecrypter: func() connection.Decrypter { + return func(c *provisioning.Connection) connection.SecureValues { + return &mockSecureValues{} + } + }, + expectedError: "invalid github connection", + }, + { + name: "error decrypting private key", + conn: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123456", + InstallationID: "789012", + }, }, }, - Secure: provisioning.ConnectionSecure{ - PrivateKey: common.InlineSecureValue{ - Name: "existing-private-key", + setupDecrypter: func() connection.Decrypter { + return func(c *provisioning.Connection) connection.SecureValues { + return &mockSecureValues{ + privateKeyErr: errors.New("failed to decrypt private key"), + } + } + }, + expectedError: "failed to decrypt private key", + }, + { + name: "error decrypting token", + conn: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", }, - Token: common.InlineSecureValue{ - Name: "existing-token", + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123456", + InstallationID: "789012", + }, }, }, - } - - mockFactory := github.NewMockGithubFactory(t) - - e := github.Extra(mockFactory) - - result, err := e.Build(ctx, conn) - require.NoError(t, err) - require.NotNil(t, result) - }) - - t.Run("should build connection with background context", func(t *testing.T) { - ctx := context.Background() - conn := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GithubConnectionType, - GitHub: &provisioning.GitHubConnectionConfig{ - AppID: "123", - InstallationID: "456", + setupDecrypter: func() connection.Decrypter { + return func(c *provisioning.Connection) connection.SecureValues { + return &mockSecureValues{ + privateKey: common.RawSecureValue("test-private-key"), + tokenErr: errors.New("failed to decrypt token"), + } + } + }, + expectedError: "failed to decrypt token", + }, + { + name: "success with empty secure values", + conn: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "123456", + InstallationID: "789012", + }, }, }, - } - - mockFactory := github.NewMockGithubFactory(t) - e := github.Extra(mockFactory) - result, err := e.Build(ctx, conn) - require.NoError(t, err) - require.NotNil(t, result) - }) - - t.Run("should always pass empty token to factory.New", func(t *testing.T) { - ctx := context.Background() - conn := &provisioning.Connection{ - ObjectMeta: metav1.ObjectMeta{Name: "test-connection"}, - Spec: provisioning.ConnectionSpec{ - Type: provisioning.GithubConnectionType, - GitHub: &provisioning.GitHubConnectionConfig{ - AppID: "123", - InstallationID: "456", + setupDecrypter: func() connection.Decrypter { + return func(c *provisioning.Connection) connection.SecureValues { + return &mockSecureValues{ + privateKey: common.RawSecureValue(""), + token: common.RawSecureValue(""), + } + } + }, + }, + { + name: "success with different app and installation IDs", + conn: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "another-connection", + Namespace: "prod", + }, + Spec: provisioning.ConnectionSpec{ + Type: provisioning.GithubConnectionType, + GitHub: &provisioning.GitHubConnectionConfig{ + AppID: "999888", + InstallationID: "777666", + }, }, }, - Secure: provisioning.ConnectionSecure{ - Token: common.InlineSecureValue{ - Create: common.NewSecretValue("some-token"), - }, + setupDecrypter: func() connection.Decrypter { + return func(c *provisioning.Connection) connection.SecureValues { + return &mockSecureValues{ + privateKey: common.RawSecureValue("another-private-key"), + token: common.RawSecureValue("another-token"), + } + } }, - } + }, + } - mockFactory := github.NewMockGithubFactory(t) - e := github.Extra(mockFactory) - result, err := e.Build(ctx, conn) - require.NoError(t, err) - require.NotNil(t, result) - }) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + mockFactory := github.NewMockGithubFactory(t) + decrypter := tt.setupDecrypter() + + e := github.Extra(decrypter, mockFactory) + + result, err := e.Build(ctx, tt.conn) + + if tt.expectedError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedError) + assert.Nil(t, result) + } else { + require.NoError(t, err) + assert.NotNil(t, result) + } + }) + } } diff --git a/apps/provisioning/pkg/connection/secure.go b/apps/provisioning/pkg/connection/secure.go new file mode 100644 index 00000000000..8cc5815df6b --- /dev/null +++ b/apps/provisioning/pkg/connection/secure.go @@ -0,0 +1,64 @@ +package connection + +import ( + "context" + "fmt" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/apps/secret/pkg/decrypt" + common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" +) + +type Decrypter = func(c *provisioning.Connection) SecureValues + +type SecureValues interface { + PrivateKey(ctx context.Context) (common.RawSecureValue, error) + ClientSecret(ctx context.Context) (common.RawSecureValue, error) + Token(ctx context.Context) (common.RawSecureValue, error) +} + +type secureValues struct { + svc decrypt.DecryptService + names provisioning.ConnectionSecure + namespace string +} + +func (s *secureValues) get(ctx context.Context, sv common.InlineSecureValue) (common.RawSecureValue, error) { + if !sv.Create.IsZero() { + return sv.Create, nil // If this was called before the value is actually saved + } + if sv.Name == "" { + return "", nil + } + results, err := s.svc.Decrypt(ctx, provisioning.GROUP, s.namespace, sv.Name) + if err != nil { + return "", fmt.Errorf("failed to call decrypt service: %w", err) + } + + v, found := results[sv.Name] + if !found { + return "", fmt.Errorf("not found") + } + if v.Error() != nil { + return "", v.Error() + } + return common.RawSecureValue(*v.Value()), nil +} + +func (s *secureValues) PrivateKey(ctx context.Context) (common.RawSecureValue, error) { + return s.get(ctx, s.names.PrivateKey) +} + +func (s *secureValues) ClientSecret(ctx context.Context) (common.RawSecureValue, error) { + return s.get(ctx, s.names.ClientSecret) +} + +func (s *secureValues) Token(ctx context.Context) (common.RawSecureValue, error) { + return s.get(ctx, s.names.Token) +} + +func ProvideDecrypter(svc decrypt.DecryptService) Decrypter { + return func(c *provisioning.Connection) SecureValues { + return &secureValues{svc: svc, names: c.Secure, namespace: c.Namespace} + } +} diff --git a/apps/provisioning/pkg/connection/secure_test.go b/apps/provisioning/pkg/connection/secure_test.go new file mode 100644 index 00000000000..cab4860d69c --- /dev/null +++ b/apps/provisioning/pkg/connection/secure_test.go @@ -0,0 +1,510 @@ +package connection_test + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/apps/provisioning/pkg/connection" + secretv1beta1 "github.com/grafana/grafana/apps/secret/pkg/apis/secret/v1beta1" + "github.com/grafana/grafana/apps/secret/pkg/decrypt" + common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" +) + +// mockDecryptService implements decrypt.DecryptService for testing +type mockDecryptService struct { + results map[string]decrypt.DecryptResult + err error +} + +func (m *mockDecryptService) Decrypt(ctx context.Context, group, namespace string, names ...string) (map[string]decrypt.DecryptResult, error) { + if m.err != nil { + return nil, m.err + } + + results := make(map[string]decrypt.DecryptResult) + for _, name := range names { + if result, ok := m.results[name]; ok { + results[name] = result + } + } + return results, nil +} + +func newDecryptResult(value string) decrypt.DecryptResult { + v := secretv1beta1.ExposedSecureValue(value) + return decrypt.NewDecryptResultValue(&v) +} + +func newDecryptResultWithError(err error) decrypt.DecryptResult { + return decrypt.NewDecryptResultErr(err) +} + +func TestProvideDecrypter(t *testing.T) { + t.Run("should return a decrypter function", func(t *testing.T) { + mockSvc := &mockDecryptService{} + decrypter := connection.ProvideDecrypter(mockSvc) + + require.NotNil(t, decrypter) + + conn := &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + } + + result := decrypter(conn) + require.NotNil(t, result) + }) +} + +func TestSecureValues_PrivateKey(t *testing.T) { + tests := []struct { + name string + connection *provisioning.Connection + mockResults map[string]decrypt.DecryptResult + mockErr error + expectedValue common.RawSecureValue + expectedError string + }{ + { + name: "returns Create value when present", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{ + Create: common.NewSecretValue("create-private-key"), + }, + }, + }, + expectedValue: common.RawSecureValue("create-private-key"), + }, + { + name: "returns empty when Name is empty", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{}, + }, + }, + expectedValue: "", + }, + { + name: "decrypts from service when Name is provided", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{ + Name: "private-key-ref", + }, + }, + }, + mockResults: map[string]decrypt.DecryptResult{ + "private-key-ref": newDecryptResult("decrypted-private-key"), + }, + expectedValue: common.RawSecureValue("decrypted-private-key"), + }, + { + name: "returns error when decrypt service fails", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{ + Name: "private-key-ref", + }, + }, + }, + mockErr: errors.New("decrypt service error"), + expectedError: "failed to call decrypt service", + }, + { + name: "returns error when value not found", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{ + Name: "missing-key", + }, + }, + }, + mockResults: map[string]decrypt.DecryptResult{}, + expectedError: "not found", + }, + { + name: "returns error when decrypt result has error", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{ + Name: "private-key-ref", + }, + }, + }, + mockResults: map[string]decrypt.DecryptResult{ + "private-key-ref": newDecryptResultWithError(errors.New("decryption failed")), + }, + expectedError: "decryption failed", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockSvc := &mockDecryptService{ + results: tt.mockResults, + err: tt.mockErr, + } + + decrypter := connection.ProvideDecrypter(mockSvc) + secureVals := decrypter(tt.connection) + + value, err := secureVals.PrivateKey(context.Background()) + + if tt.expectedError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedError) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expectedValue, value) + } + }) + } +} + +func TestSecureValues_ClientSecret(t *testing.T) { + tests := []struct { + name string + connection *provisioning.Connection + mockResults map[string]decrypt.DecryptResult + mockErr error + expectedValue common.RawSecureValue + expectedError string + }{ + { + name: "returns Create value when present", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + ClientSecret: common.InlineSecureValue{ + Create: common.NewSecretValue("create-client-secret"), + }, + }, + }, + expectedValue: common.RawSecureValue("create-client-secret"), + }, + { + name: "returns empty when Name is empty", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + ClientSecret: common.InlineSecureValue{}, + }, + }, + expectedValue: "", + }, + { + name: "decrypts from service when Name is provided", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + ClientSecret: common.InlineSecureValue{ + Name: "client-secret-ref", + }, + }, + }, + mockResults: map[string]decrypt.DecryptResult{ + "client-secret-ref": newDecryptResult("decrypted-client-secret"), + }, + expectedValue: common.RawSecureValue("decrypted-client-secret"), + }, + { + name: "returns error when decrypt service fails", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + ClientSecret: common.InlineSecureValue{ + Name: "client-secret-ref", + }, + }, + }, + mockErr: errors.New("decrypt service error"), + expectedError: "failed to call decrypt service", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockSvc := &mockDecryptService{ + results: tt.mockResults, + err: tt.mockErr, + } + + decrypter := connection.ProvideDecrypter(mockSvc) + secureVals := decrypter(tt.connection) + + value, err := secureVals.ClientSecret(context.Background()) + + if tt.expectedError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedError) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expectedValue, value) + } + }) + } +} + +func TestSecureValues_Token(t *testing.T) { + tests := []struct { + name string + connection *provisioning.Connection + mockResults map[string]decrypt.DecryptResult + mockErr error + expectedValue common.RawSecureValue + expectedError string + }{ + { + name: "returns Create value when present", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + Token: common.InlineSecureValue{ + Create: common.NewSecretValue("create-token"), + }, + }, + }, + expectedValue: common.RawSecureValue("create-token"), + }, + { + name: "returns empty when Name is empty", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + Token: common.InlineSecureValue{}, + }, + }, + expectedValue: "", + }, + { + name: "decrypts from service when Name is provided", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + Token: common.InlineSecureValue{ + Name: "token-ref", + }, + }, + }, + mockResults: map[string]decrypt.DecryptResult{ + "token-ref": newDecryptResult("decrypted-token"), + }, + expectedValue: common.RawSecureValue("decrypted-token"), + }, + { + name: "returns error when decrypt service fails", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + Token: common.InlineSecureValue{ + Name: "token-ref", + }, + }, + }, + mockErr: errors.New("decrypt service error"), + expectedError: "failed to call decrypt service", + }, + { + name: "returns error when value not found", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + Token: common.InlineSecureValue{ + Name: "missing-token", + }, + }, + }, + mockResults: map[string]decrypt.DecryptResult{}, + expectedError: "not found", + }, + { + name: "returns error when decrypt result has error", + connection: &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + Token: common.InlineSecureValue{ + Name: "token-ref", + }, + }, + }, + mockResults: map[string]decrypt.DecryptResult{ + "token-ref": newDecryptResultWithError(errors.New("decryption failed")), + }, + expectedError: "decryption failed", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockSvc := &mockDecryptService{ + results: tt.mockResults, + err: tt.mockErr, + } + + decrypter := connection.ProvideDecrypter(mockSvc) + secureVals := decrypter(tt.connection) + + value, err := secureVals.Token(context.Background()) + + if tt.expectedError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedError) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expectedValue, value) + } + }) + } +} + +func TestSecureValues_MultipleFields(t *testing.T) { + t.Run("should decrypt all fields independently", func(t *testing.T) { + conn := &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{ + Name: "private-key-ref", + }, + ClientSecret: common.InlineSecureValue{ + Name: "client-secret-ref", + }, + Token: common.InlineSecureValue{ + Name: "token-ref", + }, + }, + } + + mockSvc := &mockDecryptService{ + results: map[string]decrypt.DecryptResult{ + "private-key-ref": newDecryptResult("decrypted-private-key"), + "client-secret-ref": newDecryptResult("decrypted-client-secret"), + "token-ref": newDecryptResult("decrypted-token"), + }, + } + + decrypter := connection.ProvideDecrypter(mockSvc) + secureVals := decrypter(conn) + + privateKey, err := secureVals.PrivateKey(context.Background()) + require.NoError(t, err) + assert.Equal(t, common.RawSecureValue("decrypted-private-key"), privateKey) + + clientSecret, err := secureVals.ClientSecret(context.Background()) + require.NoError(t, err) + assert.Equal(t, common.RawSecureValue("decrypted-client-secret"), clientSecret) + + token, err := secureVals.Token(context.Background()) + require.NoError(t, err) + assert.Equal(t, common.RawSecureValue("decrypted-token"), token) + }) + + t.Run("should handle mix of Create and Name references", func(t *testing.T) { + conn := &provisioning.Connection{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-connection", + Namespace: "default", + }, + Secure: provisioning.ConnectionSecure{ + PrivateKey: common.InlineSecureValue{ + Create: common.NewSecretValue("inline-private-key"), + }, + ClientSecret: common.InlineSecureValue{ + Name: "client-secret-ref", + }, + Token: common.InlineSecureValue{ + Create: common.NewSecretValue("inline-token"), + }, + }, + } + + mockSvc := &mockDecryptService{ + results: map[string]decrypt.DecryptResult{ + "client-secret-ref": newDecryptResult("decrypted-client-secret"), + }, + } + + decrypter := connection.ProvideDecrypter(mockSvc) + secureVals := decrypter(conn) + + // PrivateKey should return Create value without calling decrypt + privateKey, err := secureVals.PrivateKey(context.Background()) + require.NoError(t, err) + assert.Equal(t, common.RawSecureValue("inline-private-key"), privateKey) + + // ClientSecret should decrypt + clientSecret, err := secureVals.ClientSecret(context.Background()) + require.NoError(t, err) + assert.Equal(t, common.RawSecureValue("decrypted-client-secret"), clientSecret) + + // Token should return Create value without calling decrypt + token, err := secureVals.Token(context.Background()) + require.NoError(t, err) + assert.Equal(t, common.RawSecureValue("inline-token"), token) + }) +} diff --git a/apps/provisioning/pkg/repository/git/repository.go b/apps/provisioning/pkg/repository/git/repository.go index bac3bd6d0a6..95543b7b851 100644 --- a/apps/provisioning/pkg/repository/git/repository.go +++ b/apps/provisioning/pkg/repository/git/repository.go @@ -101,7 +101,8 @@ func (r *gitRepository) Validate() (list field.ErrorList) { } // Readonly repositories may not need a token (if public) - if len(r.config.Spec.Workflows) > 0 { + // Also, in case a connection is provided, the token will be created by the controller. + if len(r.config.Spec.Workflows) > 0 && r.config.Spec.Connection == nil { if cfg.Token == "" && r.config.Secure.Token.IsZero() { list = append(list, field.Required(field.NewPath("secure", "token"), "a git access token is required")) } diff --git a/apps/provisioning/pkg/repository/git/repository_test.go b/apps/provisioning/pkg/repository/git/repository_test.go index 4958ab372ab..957db47f24b 100644 --- a/apps/provisioning/pkg/repository/git/repository_test.go +++ b/apps/provisioning/pkg/repository/git/repository_test.go @@ -169,6 +169,22 @@ func TestGitRepository_Validate(t *testing.T) { }, want: nil, }, + { + name: "missing token for R/W repository with connection", + config: &provisioning.Repository{ + Spec: provisioning.RepositorySpec{ + Type: "test_type", + Workflows: []provisioning.Workflow{provisioning.WriteWorkflow}, + Connection: &provisioning.ConnectionInfo{Name: "my-connection"}, + }, + }, + gitConfig: RepositoryConfig{ + URL: "https://git.example.com/repo.git", + Branch: "main", + Token: "", // Empty token - should be OK because connection is provided + }, + want: nil, + }, { name: "unsafe path", config: &provisioning.Repository{ diff --git a/apps/provisioning/pkg/repository/github/extra.go b/apps/provisioning/pkg/repository/github/extra.go index 67c156c579d..fa4ecfe3f4f 100644 --- a/apps/provisioning/pkg/repository/github/extra.go +++ b/apps/provisioning/pkg/repository/github/extra.go @@ -5,13 +5,15 @@ import ( "fmt" "github.com/grafana/grafana-app-sdk/logging" + "k8s.io/apimachinery/pkg/runtime" + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" "github.com/grafana/grafana/apps/provisioning/pkg/repository" "github.com/grafana/grafana/apps/provisioning/pkg/repository/git" "github.com/grafana/grafana/apps/provisioning/pkg/util" - "k8s.io/apimachinery/pkg/runtime" ) +//go:generate mockery --name WebhookURLBuilder --structname MockWebhookURLBuilder --inpackage --filename webhook_builder_mock.go --with-expecter type WebhookURLBuilder interface { WebhookURL(ctx context.Context, r *provisioning.Repository) string } @@ -35,24 +37,22 @@ func (e *extra) Type() provisioning.RepositoryType { } func (e *extra) Build(ctx context.Context, r *provisioning.Repository) (repository.Repository, error) { + if r == nil || r.Spec.GitHub == nil { + return nil, fmt.Errorf("github configuration is required") + } logger := logging.FromContext(ctx).With("url", r.Spec.GitHub.URL, "branch", r.Spec.GitHub.Branch, "path", r.Spec.GitHub.Path) logger.Info("Instantiating Github repository") secure := e.decrypter(r) - cfg := r.Spec.GitHub - if cfg == nil { - return nil, fmt.Errorf("github configuration is required") - } - token, err := secure.Token(ctx) if err != nil { return nil, fmt.Errorf("unable to decrypt token: %w", err) } gitRepo, err := git.NewRepository(ctx, r, git.RepositoryConfig{ - URL: cfg.URL, - Branch: cfg.Branch, - Path: cfg.Path, + URL: r.Spec.GitHub.URL, + Branch: r.Spec.GitHub.Branch, + Path: r.Spec.GitHub.Path, Token: token, }) if err != nil { diff --git a/apps/provisioning/pkg/repository/github/extra_test.go b/apps/provisioning/pkg/repository/github/extra_test.go new file mode 100644 index 00000000000..56ff14bc105 --- /dev/null +++ b/apps/provisioning/pkg/repository/github/extra_test.go @@ -0,0 +1,410 @@ +package github_test + +import ( + "context" + "errors" + "testing" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/apps/provisioning/pkg/repository" + "github.com/grafana/grafana/apps/provisioning/pkg/repository/github" + common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +type mockSecureValues struct { + token common.RawSecureValue + tokenErr error + webhookSecret common.RawSecureValue + webhookErr error +} + +func (m *mockSecureValues) Token(_ context.Context) (common.RawSecureValue, error) { + return m.token, m.tokenErr +} + +func (m *mockSecureValues) WebhookSecret(_ context.Context) (common.RawSecureValue, error) { + return m.webhookSecret, m.webhookErr +} + +func TestExtra_Type(t *testing.T) { + e := github.Extra(nil, nil, nil) + assert.Equal(t, provisioning.GitHubRepositoryType, e.Type()) +} + +func TestExtra_Build(t *testing.T) { + tests := []struct { + name string + repo *provisioning.Repository + setupDecrypter func() repository.Decrypter + setupWebhook func(t *testing.T, repo *provisioning.Repository) github.WebhookURLBuilder + expectedError string + validateResult func(t *testing.T, repo repository.Repository) + }{ + { + name: "missing github config", + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: nil, + }, + }, + setupDecrypter: func() repository.Decrypter { + return func(r *provisioning.Repository) repository.SecureValues { + return &mockSecureValues{} + } + }, + setupWebhook: func(t *testing.T, repo *provisioning.Repository) github.WebhookURLBuilder { return nil }, + expectedError: "github configuration is required", + }, + { + name: "nil repository", + repo: nil, + setupDecrypter: func() repository.Decrypter { + return func(r *provisioning.Repository) repository.SecureValues { + return &mockSecureValues{} + } + }, + setupWebhook: func(t *testing.T, repo *provisioning.Repository) github.WebhookURLBuilder { return nil }, + expectedError: "github configuration is required", + }, + { + name: "error decrypting token", + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test/repo", + Branch: "main", + }, + }, + }, + setupDecrypter: func() repository.Decrypter { + return func(r *provisioning.Repository) repository.SecureValues { + return &mockSecureValues{ + tokenErr: errors.New("decryption failed"), + } + } + }, + setupWebhook: func(t *testing.T, repo *provisioning.Repository) github.WebhookURLBuilder { return nil }, + expectedError: "unable to decrypt token", + }, + { + name: "success without webhooks", + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test/repo", + Branch: "main", + }, + }, + }, + setupDecrypter: func() repository.Decrypter { + return func(r *provisioning.Repository) repository.SecureValues { + return &mockSecureValues{ + token: common.RawSecureValue("test-token"), + } + } + }, + setupWebhook: func(t *testing.T, repo *provisioning.Repository) github.WebhookURLBuilder { + return nil + }, + validateResult: func(t *testing.T, repo repository.Repository) { + assert.NotNil(t, repo) + }, + }, + { + name: "success with webhooks", + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test/repo", + Branch: "main", + }, + }, + }, + setupDecrypter: func() repository.Decrypter { + return func(r *provisioning.Repository) repository.SecureValues { + return &mockSecureValues{ + token: common.RawSecureValue("test-token"), + webhookSecret: common.RawSecureValue("webhook-secret"), + } + } + }, + setupWebhook: func(t *testing.T, repo *provisioning.Repository) github.WebhookURLBuilder { + mockWebhook := github.NewMockWebhookURLBuilder(t) + mockWebhook.EXPECT().WebhookURL(mock.Anything, repo).Return("https://example.com/webhook") + return mockWebhook + }, + validateResult: func(t *testing.T, repo repository.Repository) { + assert.NotNil(t, repo) + }, + }, + { + name: "skip webhook setup when URL is empty", + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test/repo", + Branch: "main", + }, + }, + }, + setupDecrypter: func() repository.Decrypter { + return func(r *provisioning.Repository) repository.SecureValues { + return &mockSecureValues{ + token: common.RawSecureValue("test-token"), + } + } + }, + setupWebhook: func(t *testing.T, repo *provisioning.Repository) github.WebhookURLBuilder { + mockWebhook := github.NewMockWebhookURLBuilder(t) + mockWebhook.EXPECT().WebhookURL(mock.Anything, repo).Return("") + return mockWebhook + }, + validateResult: func(t *testing.T, repo repository.Repository) { + assert.NotNil(t, repo) + }, + }, + { + name: "error decrypting webhook secret", + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test/repo", + Branch: "main", + }, + }, + }, + setupDecrypter: func() repository.Decrypter { + return func(r *provisioning.Repository) repository.SecureValues { + return &mockSecureValues{ + token: common.RawSecureValue("test-token"), + webhookErr: errors.New("webhook decryption failed"), + } + } + }, + setupWebhook: func(t *testing.T, repo *provisioning.Repository) github.WebhookURLBuilder { + mockWebhook := github.NewMockWebhookURLBuilder(t) + mockWebhook.EXPECT().WebhookURL(mock.Anything, repo).Return("https://example.com/webhook") + return mockWebhook + }, + expectedError: "decrypt webhookSecret", + }, + { + name: "success with custom path", + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test/repo", + Branch: "main", + Path: "custom/path", + }, + }, + }, + setupDecrypter: func() repository.Decrypter { + return func(r *provisioning.Repository) repository.SecureValues { + return &mockSecureValues{ + token: common.RawSecureValue("test-token"), + } + } + }, + setupWebhook: func(t *testing.T, repo *provisioning.Repository) github.WebhookURLBuilder { + return nil + }, + validateResult: func(t *testing.T, repo repository.Repository) { + assert.NotNil(t, repo) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + decrypter := tt.setupDecrypter() + webhookBuilder := tt.setupWebhook(t, tt.repo) + factory := github.ProvideFactory() + + e := github.Extra(decrypter, factory, webhookBuilder) + + result, err := e.Build(ctx, tt.repo) + + if tt.expectedError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedError) + assert.Nil(t, result) + } else { + require.NoError(t, err) + if tt.validateResult != nil { + tt.validateResult(t, result) + } + } + }) + } +} + +func TestExtra_Mutate(t *testing.T) { + tests := []struct { + name string + obj runtime.Object + expectedError bool + validateObj func(t *testing.T, obj runtime.Object) + }{ + { + name: "mutates repository with github config", + obj: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test/repo.git/", + }, + }, + }, + expectedError: false, + validateObj: func(t *testing.T, obj runtime.Object) { + repo := obj.(*provisioning.Repository) + assert.Equal(t, "https://github.com/test/repo", repo.Spec.GitHub.URL) + }, + }, + { + name: "handles repository without github config", + obj: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: nil, + }, + }, + expectedError: false, + }, + { + name: "handles non-repository object", + obj: &runtime.Unknown{}, + expectedError: false, + }, + { + name: "trims only trailing slash", + obj: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test/repo/", + }, + }, + }, + expectedError: false, + validateObj: func(t *testing.T, obj runtime.Object) { + repo := obj.(*provisioning.Repository) + assert.Equal(t, "https://github.com/test/repo", repo.Spec.GitHub.URL) + }, + }, + { + name: "trims only .git suffix", + obj: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test/repo.git", + }, + }, + }, + expectedError: false, + validateObj: func(t *testing.T, obj runtime.Object) { + repo := obj.(*provisioning.Repository) + assert.Equal(t, "https://github.com/test/repo", repo.Spec.GitHub.URL) + }, + }, + { + name: "no changes when URL is clean", + obj: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "default", + }, + Spec: provisioning.RepositorySpec{ + Type: provisioning.GitHubRepositoryType, + GitHub: &provisioning.GitHubRepositoryConfig{ + URL: "https://github.com/test/repo", + }, + }, + }, + expectedError: false, + validateObj: func(t *testing.T, obj runtime.Object) { + repo := obj.(*provisioning.Repository) + assert.Equal(t, "https://github.com/test/repo", repo.Spec.GitHub.URL) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + + e := github.Extra(nil, nil, nil) + + err := e.Mutate(ctx, tt.obj) + + if tt.expectedError { + require.Error(t, err) + } else { + require.NoError(t, err) + if tt.validateObj != nil { + tt.validateObj(t, tt.obj) + } + } + }) + } +} diff --git a/apps/provisioning/pkg/repository/github/webhook_builder_mock.go b/apps/provisioning/pkg/repository/github/webhook_builder_mock.go new file mode 100644 index 00000000000..4505922a8c4 --- /dev/null +++ b/apps/provisioning/pkg/repository/github/webhook_builder_mock.go @@ -0,0 +1,84 @@ +// Code generated by mockery v2.53.4. DO NOT EDIT. + +package github + +import ( + context "context" + + v0alpha1 "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + mock "github.com/stretchr/testify/mock" +) + +// MockWebhookURLBuilder is an autogenerated mock type for the WebhookURLBuilder type +type MockWebhookURLBuilder struct { + mock.Mock +} + +type MockWebhookURLBuilder_Expecter struct { + mock *mock.Mock +} + +func (_m *MockWebhookURLBuilder) EXPECT() *MockWebhookURLBuilder_Expecter { + return &MockWebhookURLBuilder_Expecter{mock: &_m.Mock} +} + +// WebhookURL provides a mock function with given fields: ctx, r +func (_m *MockWebhookURLBuilder) WebhookURL(ctx context.Context, r *v0alpha1.Repository) string { + ret := _m.Called(ctx, r) + + if len(ret) == 0 { + panic("no return value specified for WebhookURL") + } + + var r0 string + if rf, ok := ret.Get(0).(func(context.Context, *v0alpha1.Repository) string); ok { + r0 = rf(ctx, r) + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + +// MockWebhookURLBuilder_WebhookURL_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'WebhookURL' +type MockWebhookURLBuilder_WebhookURL_Call struct { + *mock.Call +} + +// WebhookURL is a helper method to define mock.On call +// - ctx context.Context +// - r *v0alpha1.Repository +func (_e *MockWebhookURLBuilder_Expecter) WebhookURL(ctx interface{}, r interface{}) *MockWebhookURLBuilder_WebhookURL_Call { + return &MockWebhookURLBuilder_WebhookURL_Call{Call: _e.mock.On("WebhookURL", ctx, r)} +} + +func (_c *MockWebhookURLBuilder_WebhookURL_Call) Run(run func(ctx context.Context, r *v0alpha1.Repository)) *MockWebhookURLBuilder_WebhookURL_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*v0alpha1.Repository)) + }) + return _c +} + +func (_c *MockWebhookURLBuilder_WebhookURL_Call) Return(_a0 string) *MockWebhookURLBuilder_WebhookURL_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockWebhookURLBuilder_WebhookURL_Call) RunAndReturn(run func(context.Context, *v0alpha1.Repository) string) *MockWebhookURLBuilder_WebhookURL_Call { + _c.Call.Return(run) + return _c +} + +// NewMockWebhookURLBuilder creates a new instance of MockWebhookURLBuilder. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewMockWebhookURLBuilder(t interface { + mock.TestingT + Cleanup(func()) +}) *MockWebhookURLBuilder { + mock := &MockWebhookURLBuilder{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/registry/apis/provisioning/extras/register.go b/pkg/registry/apis/provisioning/extras/register.go index 43c7064173f..901b6bd6169 100644 --- a/pkg/registry/apis/provisioning/extras/register.go +++ b/pkg/registry/apis/provisioning/extras/register.go @@ -46,10 +46,12 @@ func ProvideProvisioningOSSRepositoryExtras( func ProvideProvisioningOSSConnectionExtras( _ *setting.Cfg, + decryptSvc decrypt.DecryptService, ghFactory ghconnection.GithubFactory, ) []connection.Extra { + decrypter := connection.ProvideDecrypter(decryptSvc) return []connection.Extra{ - ghconnection.Extra(ghFactory), + ghconnection.Extra(decrypter, ghFactory), } } diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 901bc829c97..4b79e75baf0 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -97,7 +97,8 @@ type APIBuilder struct { usageStats usagestats.Service tracer tracing.Tracer - store grafanarest.Storage + repoStore grafanarest.Storage + connectionStore grafanarest.Storage parsers resources.ParserFactory repositoryResources resources.RepositoryResourcesFactory clients resources.ClientFactory @@ -604,7 +605,7 @@ func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupI } repositoryStatusStorage := grafanaregistry.NewRegistryStatusStore(opts.Scheme, repositoryStorage) - b.store = repositoryStorage + b.repoStore = repositoryStorage jobStore, err := grafanaregistry.NewCompleteRegistryStore(opts.Scheme, provisioning.JobResourceInfo, opts.OptsGetter) if err != nil { @@ -636,6 +637,7 @@ func (b *APIBuilder) UpdateAPIGroupInfo(apiGroupInfo *genericapiserver.APIGroupI return fmt.Errorf("failed to create connection storage: %w", err) } connectionStatusStorage := grafanaregistry.NewRegistryStatusStore(opts.Scheme, connectionsStore) + b.connectionStore = connectionsStore storage[provisioning.JobResourceInfo.StoragePath()] = jobStore storage[provisioning.RepositoryResourceInfo.StoragePath()] = repositoryStorage @@ -817,7 +819,7 @@ func invalidRepositoryError(name string, list field.ErrorList) error { } func (b *APIBuilder) VerifyAgainstExistingRepositories(ctx context.Context, cfg *provisioning.Repository) *field.Error { - return VerifyAgainstExistingRepositories(ctx, b.store, cfg) + return VerifyAgainstExistingRepositories(ctx, b.repoStore, cfg) } func (b *APIBuilder) GetPostStartHooks() (map[string]genericapiserver.PostStartHookFunc, error) { @@ -865,7 +867,7 @@ func (b *APIBuilder) GetPostStartHooks() (map[string]genericapiserver.PostStartH // Create the repository resources factory repositoryListerWrapper := func(ctx context.Context) ([]provisioning.Repository, error) { - return GetRepositoriesInNamespace(ctx, b.store) + return GetRepositoriesInNamespace(ctx, b.repoStore) } usageMetricCollector := usage.MetricCollector(b.tracer, repositoryListerWrapper, b.unified) b.usageStats.RegisterMetricsFunc(usageMetricCollector) @@ -1411,13 +1413,21 @@ spec: // TODO: where should the helpers live? func (b *APIBuilder) GetRepository(ctx context.Context, name string) (repository.Repository, error) { - obj, err := b.store.Get(ctx, name, &metav1.GetOptions{}) + obj, err := b.repoStore.Get(ctx, name, &metav1.GetOptions{}) if err != nil { return nil, err } return b.asRepository(ctx, obj, nil) } +func (b *APIBuilder) GetConnection(ctx context.Context, name string) (connection.Connection, error) { + obj, err := b.connectionStore.Get(ctx, name, &metav1.GetOptions{}) + if err != nil { + return nil, err + } + return b.asConnection(ctx, obj, nil) +} + func (b *APIBuilder) GetRepoFactory() repository.Factory { return b.repoFactory } diff --git a/pkg/registry/apis/provisioning/routes.go b/pkg/registry/apis/provisioning/routes.go index c818a76812b..2cb412c4e86 100644 --- a/pkg/registry/apis/provisioning/routes.go +++ b/pkg/registry/apis/provisioning/routes.go @@ -149,7 +149,7 @@ func (b *APIBuilder) handleSettings(w http.ResponseWriter, r *http.Request) { } // TODO: check if lister could list too many repositories or resources - all, err := GetRepositoriesInNamespace(request.WithNamespace(r.Context(), u.GetNamespace()), b.store) + all, err := GetRepositoriesInNamespace(request.WithNamespace(r.Context(), u.GetNamespace()), b.repoStore) if err != nil { errhttp.Write(r.Context(), err, w) return diff --git a/pkg/registry/apis/provisioning/test.go b/pkg/registry/apis/provisioning/test.go index 43e28bdb334..482467c94c7 100644 --- a/pkg/registry/apis/provisioning/test.go +++ b/pkg/registry/apis/provisioning/test.go @@ -30,23 +30,29 @@ type HealthCheckerProvider interface { type ConnectorDependencies interface { RepoGetter + ConnectionGetter HealthCheckerProvider GetRepoFactory() repository.Factory } type testConnector struct { - getter RepoGetter - factory repository.Factory - healthProvider HealthCheckerProvider - tester repository.RepositoryTesterWithExistingChecker + repoGetter RepoGetter + repoFactory repository.Factory + connectionGetter ConnectionGetter + healthProvider HealthCheckerProvider + tester repository.RepositoryTesterWithExistingChecker } -func NewTestConnector(deps ConnectorDependencies, tester repository.RepositoryTesterWithExistingChecker) *testConnector { +func NewTestConnector( + deps ConnectorDependencies, + tester repository.RepositoryTesterWithExistingChecker, +) *testConnector { return &testConnector{ - factory: deps.GetRepoFactory(), - getter: deps, - healthProvider: deps, - tester: tester, + repoFactory: deps.GetRepoFactory(), + repoGetter: deps, + connectionGetter: deps, + healthProvider: deps, + tester: tester, } } @@ -72,7 +78,7 @@ func (*testConnector) NewConnectOptions() (runtime.Object, bool, string) { return nil, false, "" } -func (s *testConnector) Connect(ctx context.Context, name string, opts runtime.Object, responder rest.Responder) (http.Handler, error) { +func (s *testConnector) Connect(ctx context.Context, name string, _ runtime.Object, responder rest.Responder) (http.Handler, error) { ns, ok := request.NamespaceFrom(ctx) if !ok { return nil, fmt.Errorf("missing namespace") @@ -104,7 +110,7 @@ func (s *testConnector) Connect(ctx context.Context, name string, opts runtime.O name = "hack-on-hack-for-new" } else { // Copy previous secure values if they exist - old, _ := s.getter.GetRepository(ctx, name) + old, _ := s.repoGetter.GetRepository(ctx, name) if old != nil && !old.Config().Secure.IsZero() { secure := old.Config().Secure if cfg.Secure.Token.IsZero() { @@ -121,8 +127,27 @@ func (s *testConnector) Connect(ctx context.Context, name string, opts runtime.O cfg.SetNamespace(ns) } + // The new repository should be connected to a Connection resource, + // i.e. we should be generating the token based on it. + if cfg.Secure.Token.IsZero() && cfg.Spec.Connection != nil && cfg.Spec.Connection.Name != "" { + // A connection must be there + c, err := s.connectionGetter.GetConnection(ctx, cfg.Spec.Connection.Name) + if err != nil { + responder.Error(err) + return + } + + token, err := c.GenerateRepositoryToken(ctx, &cfg) + if err != nil { + responder.Error(err) + return + } + + cfg.Secure.Token.Create = token + } + // Create a temporary repository - tmp, err := s.factory.Build(ctx, &cfg) + tmp, err := s.repoFactory.Build(ctx, &cfg) if err != nil { responder.Error(err) return @@ -148,7 +173,7 @@ func (s *testConnector) Connect(ctx context.Context, name string, opts runtime.O } // Testing existing repository - get it and update health - repo, err = s.getter.GetRepository(ctx, name) + repo, err = s.repoGetter.GetRepository(ctx, name) if err != nil { responder.Error(err) return diff --git a/pkg/registry/apis/provisioning/types.go b/pkg/registry/apis/provisioning/types.go index f39d9ae10ce..936bed9f43c 100644 --- a/pkg/registry/apis/provisioning/types.go +++ b/pkg/registry/apis/provisioning/types.go @@ -3,6 +3,7 @@ package provisioning import ( "context" + "github.com/grafana/grafana/apps/provisioning/pkg/connection" client "github.com/grafana/grafana/apps/provisioning/pkg/generated/clientset/versioned/typed/provisioning/v0alpha1" "github.com/grafana/grafana/apps/provisioning/pkg/repository" ) @@ -15,6 +16,10 @@ type RepoGetter interface { GetHealthyRepository(ctx context.Context, name string) (repository.Repository, error) } +type ConnectionGetter interface { + GetConnection(ctx context.Context, name string) (connection.Connection, error) +} + type ClientGetter interface { GetClient() client.ProvisioningV0alpha1Interface } diff --git a/pkg/server/wire_gen.go b/pkg/server/wire_gen.go index 218e9fabc36..89d0008b434 100644 --- a/pkg/server/wire_gen.go +++ b/pkg/server/wire_gen.go @@ -916,7 +916,7 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api return nil, err } githubFactory := github2.ProvideFactory() - v7 := extras.ProvideProvisioningOSSConnectionExtras(cfg, githubFactory) + v7 := extras.ProvideProvisioningOSSConnectionExtras(cfg, decryptService, githubFactory) connectionFactory, err := extras.ProvideConnectionFactoryFromConfig(cfg, v7) if err != nil { return nil, err @@ -1584,7 +1584,7 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac return nil, err } githubFactory := github2.ProvideFactory() - v7 := extras.ProvideProvisioningOSSConnectionExtras(cfg, githubFactory) + v7 := extras.ProvideProvisioningOSSConnectionExtras(cfg, decryptService, githubFactory) connectionFactory, err := extras.ProvideConnectionFactoryFromConfig(cfg, v7) if err != nil { return nil, err