Provisioning: Block connection deletion when repositories reference it

This change prevents deletion of Connections when any Repository
references them via spec.connection.name. This uses the field selector
feature added in the previous commit.

Implementation:
- Add GetRepositoriesByConnection function to query repositories by
  connection name using the spec.connection.name field selector
- Add validateDelete method to handle Connection deletion validation
- Return Forbidden error with list of connected repository names

Closes: https://github.com/grafana/git-ui-sync-project/issues/730
This commit is contained in:
Roberto Jimenez Sanchez
2026-01-09 16:34:41 +01:00
parent f625902e4b
commit ba12ac68cc
4 changed files with 478 additions and 1 deletions
+43 -1
View File
@@ -720,7 +720,13 @@ func (b *APIBuilder) Mutate(ctx context.Context, a admission.Attributes, o admis
// TODO: move logic to a more appropriate place. Probably controller/validation.go
func (b *APIBuilder) Validate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) {
obj := a.GetObject()
if obj == nil || a.GetOperation() == admission.Connect || a.GetOperation() == admission.Delete {
// Handle Connection deletion - check for connected repositories
if a.GetOperation() == admission.Delete {
return b.validateDelete(ctx, a)
}
if obj == nil || a.GetOperation() == admission.Connect {
return nil // This is normal for sub-resource
}
@@ -800,6 +806,42 @@ func invalidRepositoryError(name string, list field.ErrorList) error {
name, list)
}
// validateDelete handles validation for delete operations
func (b *APIBuilder) validateDelete(ctx context.Context, a admission.Attributes) error {
// Only validate Connection deletions
if a.GetResource().Resource != "connections" {
return nil
}
connectionName := a.GetName()
namespace := a.GetNamespace()
// Set namespace in context for the repository store query
ctx, _, err := identity.WithProvisioningIdentity(ctx, namespace)
if err != nil {
return apierrors.NewInternalError(fmt.Errorf("failed to set provisioning identity: %w", err))
}
repos, err := GetRepositoriesByConnection(ctx, b.store, connectionName)
if err != nil {
return apierrors.NewInternalError(fmt.Errorf("failed to check for connected repositories: %w", err))
}
if len(repos) > 0 {
repoNames := make([]string, 0, len(repos))
for _, repo := range repos {
repoNames = append(repoNames, repo.Name)
}
return apierrors.NewForbidden(
provisioning.ConnectionResourceInfo.GroupResource(),
connectionName,
fmt.Errorf("cannot delete connection while repositories are using it: %s", strings.Join(repoNames, ", ")),
)
}
return nil
}
func (b *APIBuilder) VerifyAgainstExistingRepositories(ctx context.Context, cfg *provisioning.Repository) *field.Error {
return VerifyAgainstExistingRepositories(ctx, b.store, cfg)
}
@@ -7,6 +7,7 @@ import (
"strings"
"k8s.io/apimachinery/pkg/apis/meta/internalversion"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/endpoints/request"
@@ -50,6 +51,39 @@ func GetRepositoriesInNamespace(ctx context.Context, store RepositoryLister) ([]
return allRepositories, nil
}
// GetRepositoriesByConnection retrieves all repositories that reference a specific connection
func GetRepositoriesByConnection(ctx context.Context, store RepositoryLister, connectionName string) ([]provisioning.Repository, error) {
var allRepositories []provisioning.Repository
continueToken := ""
fieldSelector := fields.OneTermEqualSelector("spec.connection.name", connectionName)
for {
obj, err := store.List(ctx, &internalversion.ListOptions{
Limit: 100,
Continue: continueToken,
FieldSelector: fieldSelector,
})
if err != nil {
return nil, err
}
repositoryList, ok := obj.(*provisioning.RepositoryList)
if !ok {
return nil, fmt.Errorf("expected repository list")
}
allRepositories = append(allRepositories, repositoryList.Items...)
continueToken = repositoryList.GetContinue()
if continueToken == "" {
break
}
}
return allRepositories, nil
}
// VerifyAgainstExistingRepositories validates a repository configuration against existing repositories
func VerifyAgainstExistingRepositories(ctx context.Context, store RepositoryLister, cfg *provisioning.Repository) *field.Error {
ctx, _, err := identity.WithProvisioningIdentity(ctx, cfg.Namespace)
@@ -0,0 +1,200 @@
package provisioning
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/apis/meta/internalversion"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/runtime"
provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1"
)
// mockRepositoryLister is a mock implementation of RepositoryLister for testing
type mockRepositoryLister struct {
repositories []provisioning.Repository
listErr error
// Track the field selector used in List calls
lastFieldSelector fields.Selector
}
func (m *mockRepositoryLister) List(ctx context.Context, options *internalversion.ListOptions) (runtime.Object, error) {
if m.listErr != nil {
return nil, m.listErr
}
// Store the field selector for verification
m.lastFieldSelector = options.FieldSelector
// Filter repositories based on field selector if present
filteredRepos := m.repositories
if options.FieldSelector != nil && !options.FieldSelector.Empty() {
filteredRepos = make([]provisioning.Repository, 0)
for _, repo := range m.repositories {
// Simulate field selector matching for spec.connection.name
repoFields := fields.Set{
"spec.connection.name": getRepoConnectionName(&repo),
}
if options.FieldSelector.Matches(repoFields) {
filteredRepos = append(filteredRepos, repo)
}
}
}
return &provisioning.RepositoryList{
Items: filteredRepos,
}, nil
}
func getRepoConnectionName(repo *provisioning.Repository) string {
if repo.Spec.Connection == nil {
return ""
}
return repo.Spec.Connection.Name
}
func TestGetRepositoriesByConnection(t *testing.T) {
tests := []struct {
name string
repositories []provisioning.Repository
connectionName string
expectedCount int
expectedNames []string
expectedErr bool
}{
{
name: "empty repository list returns empty",
repositories: []provisioning.Repository{},
connectionName: "test-conn",
expectedCount: 0,
expectedNames: []string{},
},
{
name: "finds single matching repository",
repositories: []provisioning.Repository{
{
ObjectMeta: metav1.ObjectMeta{Name: "repo-1"},
Spec: provisioning.RepositorySpec{
Connection: &provisioning.ConnectionInfo{Name: "conn-a"},
},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "repo-2"},
Spec: provisioning.RepositorySpec{
Connection: &provisioning.ConnectionInfo{Name: "conn-b"},
},
},
},
connectionName: "conn-a",
expectedCount: 1,
expectedNames: []string{"repo-1"},
},
{
name: "finds multiple matching repositories",
repositories: []provisioning.Repository{
{
ObjectMeta: metav1.ObjectMeta{Name: "repo-1"},
Spec: provisioning.RepositorySpec{
Connection: &provisioning.ConnectionInfo{Name: "shared-conn"},
},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "repo-2"},
Spec: provisioning.RepositorySpec{
Connection: &provisioning.ConnectionInfo{Name: "shared-conn"},
},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "repo-3"},
Spec: provisioning.RepositorySpec{
Connection: &provisioning.ConnectionInfo{Name: "different-conn"},
},
},
},
connectionName: "shared-conn",
expectedCount: 2,
expectedNames: []string{"repo-1", "repo-2"},
},
{
name: "no matches returns empty list",
repositories: []provisioning.Repository{
{
ObjectMeta: metav1.ObjectMeta{Name: "repo-1"},
Spec: provisioning.RepositorySpec{
Connection: &provisioning.ConnectionInfo{Name: "conn-a"},
},
},
},
connectionName: "non-existent",
expectedCount: 0,
expectedNames: []string{},
},
{
name: "empty connection name matches repos without connection",
repositories: []provisioning.Repository{
{
ObjectMeta: metav1.ObjectMeta{Name: "repo-with-conn"},
Spec: provisioning.RepositorySpec{
Connection: &provisioning.ConnectionInfo{Name: "some-conn"},
},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "repo-without-conn"},
Spec: provisioning.RepositorySpec{
Connection: nil,
},
},
},
connectionName: "",
expectedCount: 1,
expectedNames: []string{"repo-without-conn"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mock := &mockRepositoryLister{repositories: tt.repositories}
ctx := context.Background()
repos, err := GetRepositoriesByConnection(ctx, mock, tt.connectionName)
if tt.expectedErr {
require.Error(t, err)
return
}
require.NoError(t, err)
assert.Len(t, repos, tt.expectedCount)
// Verify the field selector was used
require.NotNil(t, mock.lastFieldSelector, "field selector should have been set")
expectedSelector := fields.OneTermEqualSelector("spec.connection.name", tt.connectionName)
assert.Equal(t, expectedSelector.String(), mock.lastFieldSelector.String())
// Verify the correct repositories were returned
actualNames := make([]string, len(repos))
for i, repo := range repos {
actualNames[i] = repo.Name
}
for _, expectedName := range tt.expectedNames {
assert.Contains(t, actualNames, expectedName)
}
})
}
}
func TestGetRepositoriesByConnection_ListError(t *testing.T) {
mock := &mockRepositoryLister{
listErr: assert.AnError,
}
ctx := context.Background()
repos, err := GetRepositoriesByConnection(ctx, mock, "any-conn")
require.Error(t, err)
assert.Nil(t, repos)
}
@@ -731,3 +731,204 @@ func TestIntegrationProvisioning_RepositoryFieldSelectorByConnection(t *testing.
assert.Contains(t, names, "repo-with-different-connection")
})
}
func TestIntegrationProvisioning_ConnectionDeletionBlocking(t *testing.T) {
testutil.SkipIntegrationTestInShortMode(t)
helper := runGrafana(t)
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
t.Cleanup(cancel)
createOptions := metav1.CreateOptions{}
// Create a connection for testing deletion blocking
connName := "test-conn-delete-blocking"
_, err := helper.Connections.Resource.Create(ctx, &unstructured.Unstructured{
Object: map[string]any{
"apiVersion": "provisioning.grafana.app/v0alpha1",
"kind": "Connection",
"metadata": map[string]any{
"name": connName,
"namespace": "default",
},
"spec": map[string]any{
"type": "github",
"github": map[string]any{
"appID": "123456",
"installationID": "454545",
},
},
"secure": map[string]any{
"privateKey": map[string]any{
"create": "someSecret",
},
},
},
}, createOptions)
require.NoError(t, err, "failed to create test connection")
t.Run("delete connection without connected repositories succeeds", func(t *testing.T) {
// Create a connection that has no repositories
emptyConnName := "test-conn-no-repos"
_, err := helper.Connections.Resource.Create(ctx, &unstructured.Unstructured{
Object: map[string]any{
"apiVersion": "provisioning.grafana.app/v0alpha1",
"kind": "Connection",
"metadata": map[string]any{
"name": emptyConnName,
"namespace": "default",
},
"spec": map[string]any{
"type": "github",
"github": map[string]any{
"appID": "123457",
"installationID": "454546",
},
},
"secure": map[string]any{
"privateKey": map[string]any{
"create": "someSecret",
},
},
},
}, createOptions)
require.NoError(t, err, "failed to create test connection without repos")
// Delete should succeed since no repositories reference it
err = helper.Connections.Resource.Delete(ctx, emptyConnName, metav1.DeleteOptions{})
require.NoError(t, err, "deleting connection without connected repositories should succeed")
// Verify the connection is deleted
_, err = helper.Connections.Resource.Get(ctx, emptyConnName, metav1.GetOptions{})
require.True(t, k8serrors.IsNotFound(err), "connection should be deleted")
})
t.Run("delete connection with connected repository fails", func(t *testing.T) {
// Create a repository that uses the connection
repoName := "repo-using-connection"
_, err := helper.Repositories.Resource.Create(ctx, &unstructured.Unstructured{
Object: map[string]any{
"apiVersion": "provisioning.grafana.app/v0alpha1",
"kind": "Repository",
"metadata": map[string]any{
"name": repoName,
"namespace": "default",
},
"spec": map[string]any{
"title": "Test Repository",
"type": "local",
"sync": map[string]any{
"enabled": false,
"target": "folder",
},
"local": map[string]any{
"path": helper.ProvisioningPath,
},
"connection": map[string]any{
"name": connName,
},
},
},
}, createOptions)
require.NoError(t, err, "failed to create repository using connection")
// Attempt to delete the connection - should fail
err = helper.Connections.Resource.Delete(ctx, connName, metav1.DeleteOptions{})
require.Error(t, err, "deleting connection with connected repository should fail")
require.True(t, k8serrors.IsForbidden(err), "error should be Forbidden, got: %v", err)
assert.Contains(t, err.Error(), repoName, "error should mention the connected repository name")
assert.Contains(t, err.Error(), "cannot delete connection while repositories are using it", "error should explain why deletion is blocked")
// Clean up: delete the repository first
err = helper.Repositories.Resource.Delete(ctx, repoName, metav1.DeleteOptions{})
require.NoError(t, err, "failed to delete test repository")
// Wait for the repository to be deleted
require.Eventually(t, func() bool {
_, err := helper.Repositories.Resource.Get(ctx, repoName, metav1.GetOptions{})
return k8serrors.IsNotFound(err)
}, 10*time.Second, 100*time.Millisecond, "repository should be deleted")
})
t.Run("delete connection after disconnecting repository succeeds", func(t *testing.T) {
// Now that the repository is deleted, the connection should be deletable
err = helper.Connections.Resource.Delete(ctx, connName, metav1.DeleteOptions{})
require.NoError(t, err, "deleting connection after removing connected repositories should succeed")
// Verify the connection is deleted
_, err = helper.Connections.Resource.Get(ctx, connName, metav1.GetOptions{})
require.True(t, k8serrors.IsNotFound(err), "connection should be deleted")
})
t.Run("delete connection with multiple connected repositories lists all", func(t *testing.T) {
// Create a new connection
multiConnName := "test-conn-multi-repos"
_, err := helper.Connections.Resource.Create(ctx, &unstructured.Unstructured{
Object: map[string]any{
"apiVersion": "provisioning.grafana.app/v0alpha1",
"kind": "Connection",
"metadata": map[string]any{
"name": multiConnName,
"namespace": "default",
},
"spec": map[string]any{
"type": "github",
"github": map[string]any{
"appID": "123458",
"installationID": "454547",
},
},
"secure": map[string]any{
"privateKey": map[string]any{
"create": "someSecret",
},
},
},
}, createOptions)
require.NoError(t, err, "failed to create multi-repo test connection")
// Create multiple repositories using this connection
repoNames := []string{"multi-repo-1", "multi-repo-2"}
for _, repoName := range repoNames {
_, err := helper.Repositories.Resource.Create(ctx, &unstructured.Unstructured{
Object: map[string]any{
"apiVersion": "provisioning.grafana.app/v0alpha1",
"kind": "Repository",
"metadata": map[string]any{
"name": repoName,
"namespace": "default",
},
"spec": map[string]any{
"title": "Test Repository " + repoName,
"type": "local",
"sync": map[string]any{
"enabled": false,
"target": "folder",
},
"local": map[string]any{
"path": helper.ProvisioningPath,
},
"connection": map[string]any{
"name": multiConnName,
},
},
},
}, createOptions)
require.NoError(t, err, "failed to create repository %s", repoName)
}
// Attempt to delete - should fail and list all repos
err = helper.Connections.Resource.Delete(ctx, multiConnName, metav1.DeleteOptions{})
require.Error(t, err, "deleting connection with multiple repos should fail")
require.True(t, k8serrors.IsForbidden(err), "error should be Forbidden")
for _, repoName := range repoNames {
assert.Contains(t, err.Error(), repoName, "error should mention repository %s", repoName)
}
// Clean up
for _, repoName := range repoNames {
_ = helper.Repositories.Resource.Delete(ctx, repoName, metav1.DeleteOptions{})
}
_ = helper.Connections.Resource.Delete(ctx, multiConnName, metav1.DeleteOptions{})
})
}