public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Provisioning: delete secrets on repository deletion (#108113)

Browse Source 
- Add hooks to git, github and github webhooks to remove the. 
- Implement deletion in secrets package.
- Add `Mutator` interface and hooks so that we can register any mutator. 
- Add unit test coverage to those mutators. 
- Move provider specific mutation from the massive `register.go` to the respective packages (e.g. `git` , `github`, etc). 
- Add integration test for removal. 
- Change the decryption fallback to simply check for the repository prefix.
This commit is contained in:
Roberto Jiménez Sánchez
2025-07-16 07:38:42 +00:00
committed by GitHub
parent 4a779c4ccb
commit 56543db16a
27 changed files with 1956 additions and 174 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/registry/apis/provisioning/webhooks/mutator.go
+35
View File
@@ -0,0 +1,35 @@
package webhooks
import (
"context"
provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/controller"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/secrets"
"k8s.io/apimachinery/pkg/runtime"
)
func Mutator(secrets secrets.RepositorySecrets) controller.Mutator {
return func(ctx context.Context, obj runtime.Object) error {
repo, ok := obj.(*provisioning.Repository)
if !ok {
return nil
}
if repo.Status.Webhook == nil {
return nil
}
if repo.Status.Webhook.Secret != "" {
secretName := repo.Name + webhookSecretSuffix
nameOrValue, err := secrets.Encrypt(ctx, repo, secretName, repo.Status.Webhook.Secret)
if err != nil {
return err
}
repo.Status.Webhook.EncryptedSecret = nameOrValue
repo.Status.Webhook.Secret = ""
}
return nil
}
}
pkg/registry/apis/provisioning/webhooks/mutator_test.go
+157
View File
@@ -0,0 +1,157 @@
package webhooks
import (
"context"
"errors"
"testing"
provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/secrets"
"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
)
func TestMutator(t *testing.T) {
tests := []struct {
name string
obj runtime.Object
secret string
setupMocks func(*secrets.MockRepositorySecrets)
expectedEncryptedSecret string
expectedError string
}{
{
name: "successful secret encryption",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Status: provisioning.RepositoryStatus{
Webhook: &provisioning.WebhookStatus{
Secret: "webhook-secret",
},
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {
mockSecrets.EXPECT().Encrypt(
context.Background(),
&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Status: provisioning.RepositoryStatus{
Webhook: &provisioning.WebhookStatus{
Secret: "webhook-secret",
},
},
},
"test-repo-webhook-secret",
"webhook-secret",
).Return([]byte("encrypted-webhook-secret"), nil)
},
expectedEncryptedSecret: "encrypted-webhook-secret",
},
{
name: "encryption error",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Status: provisioning.RepositoryStatus{
Webhook: &provisioning.WebhookStatus{
Secret: "webhook-secret",
},
},
},
setupMocks: func(mockSecrets *secrets.MockRepositorySecrets) {
mockSecrets.EXPECT().Encrypt(
context.Background(),
&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Status: provisioning.RepositoryStatus{
Webhook: &provisioning.WebhookStatus{
Secret: "webhook-secret",
},
},
},
"test-repo-webhook-secret",
"webhook-secret",
).Return(nil, errors.New("encryption failed"))
},
expectedError: "encryption failed",
},
{
name: "no webhook status",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Status: provisioning.RepositoryStatus{
Webhook: nil,
},
},
setupMocks: func(_ *secrets.MockRepositorySecrets) {
// No expectations
},
},
{
name: "empty secret",
obj: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Status: provisioning.RepositoryStatus{
Webhook: &provisioning.WebhookStatus{
Secret: "",
},
},
},
setupMocks: func(_ *secrets.MockRepositorySecrets) {
// No expectations
},
},
{
name: "non-repository object",
obj: &runtime.Unknown{},
setupMocks: func(_ *secrets.MockRepositorySecrets) {
// No expectations
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mockSecrets := secrets.NewMockRepositorySecrets(t)
tt.setupMocks(mockSecrets)
mutator := Mutator(mockSecrets)
err := mutator(context.Background(), tt.obj)
if tt.expectedError != "" {
assert.Error(t, err)
assert.Contains(t, err.Error(), tt.expectedError)
} else {
assert.NoError(t, err)
// Check that secret was cleared and encrypted secret was set
if repo, ok := tt.obj.(*provisioning.Repository); ok && repo.Status.Webhook != nil {
if tt.expectedEncryptedSecret != "" {
// Secret should be cleared after encryption
assert.Empty(t, repo.Status.Webhook.Secret, "Secret should be cleared after encryption")
// EncryptedSecret should be set to the expected value
assert.Equal(t, tt.expectedEncryptedSecret, string(repo.Status.Webhook.EncryptedSecret), "EncryptedSecret should match expected value")
}
}
}
})
}
}
pkg/registry/apis/provisioning/webhooks/register.go
+7 -16
View File
@@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana-app-sdk/logging"
provisioning "github.com/grafana/grafana/pkg/apis/provisioning/v0alpha1"
provisioningapis "github.com/grafana/grafana/pkg/registry/apis/provisioning"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/controller"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/repository"
"github.com/grafana/grafana/pkg/registry/apis/provisioning/repository/git"
@@ -128,21 +129,11 @@ func (e *WebhookExtra) Authorize(ctx context.Context, a authorizer.Attributes) (
return e.render.Authorize(ctx, a)
}
// Mutate delegates mutation to the webhook connector
func (e *WebhookExtra) Mutate(ctx context.Context, r *provisioning.Repository) error {
// Encrypt webhook secret if present
if r.Status.Webhook != nil && r.Status.Webhook.Secret != "" {
secretName := r.GetName() + "-webhook-secret"
nameOrValue, err := e.secrets.Encrypt(ctx, r, secretName, r.Status.Webhook.Secret)
if err != nil {
return fmt.Errorf("failed to encrypt webhook secret: %w", err)
}
r.Status.Webhook.EncryptedSecret = nameOrValue
r.Status.Webhook.Secret = ""
// Mutators returns the mutators for the webhook extra
func (e *WebhookExtra) Mutators() []controller.Mutator {
return []controller.Mutator{
Mutator(e.secrets),
}
return nil
}
// UpdateStorage updates the storage with both render and webhook connectors
@@ -206,12 +197,12 @@ func (e *WebhookExtra) AsRepository(ctx context.Context, r *provisioning.Reposit
EncryptedToken: ghCfg.EncryptedToken,
}
gitRepo, err := git.NewGitRepository(ctx, r, gitCfg)
gitRepo, err := git.NewGitRepository(ctx, r, gitCfg, e.secrets)
if err != nil {
return nil, fmt.Errorf("error creating git repository: %w", err)
}
basicRepo, err := github.NewGitHub(ctx, r, gitRepo, e.ghFactory, ghToken)
basicRepo, err := github.NewGitHub(ctx, r, gitRepo, e.ghFactory, ghToken, e.secrets)
if err != nil {
return nil, fmt.Errorf("error creating github repository: %w", err)
}
pkg/registry/apis/provisioning/webhooks/repository.go
+19 -3
View File
@@ -20,6 +20,9 @@ import (
var subscribedEvents = []string{"push", "pull_request"}
//nolint:gosec // This is a constant for a secret suffix
const webhookSecretSuffix = "-webhook-secret"
type WebhookRepository interface {
Webhook(ctx context.Context, req *http.Request) (*provisioning.WebhookResponse, error)
}
@@ -269,6 +272,7 @@ func (r *githubWebhookRepository) updateWebhook(ctx context.Context) (pgh.Webhoo
}
func (r *githubWebhookRepository) deleteWebhook(ctx context.Context) error {
logger := logging.FromContext(ctx)
if r.config.Status.Webhook == nil {
return fmt.Errorf("webhook not found")
}
@@ -279,7 +283,7 @@ func (r *githubWebhookRepository) deleteWebhook(ctx context.Context) error {
return fmt.Errorf("delete webhook: %w", err)
}
logging.FromContext(ctx).Info("webhook deleted", "url", r.config.Status.Webhook.URL, "id", id)
logger.Info("webhook deleted", "url", r.config.Status.Webhook.URL, "id", id)
return nil
}
@@ -332,10 +336,22 @@ func (r *githubWebhookRepository) OnUpdate(ctx context.Context) ([]map[string]in
}
func (r *githubWebhookRepository) OnDelete(ctx context.Context) error {
if len(r.webhookURL) == 0 {
ctx, logger := r.logger(ctx, "")
if err := r.GithubRepository.OnDelete(ctx); err != nil {
return fmt.Errorf("on delete from basic github repository: %w", err)
}
if r.config.Status.Webhook == nil {
return nil
}
ctx, _ = r.logger(ctx, "")
secretName := r.config.Name + webhookSecretSuffix
if err := r.secrets.Delete(ctx, r.config, secretName); err != nil {
return fmt.Errorf("delete webhook secret: %w", err)
}
logger.Info("Deleted webhook secret", "secretName", secretName)
return r.deleteWebhook(ctx)
}
pkg/registry/apis/provisioning/webhooks/repository_test.go
+219 -15
View File
@@ -1687,19 +1687,24 @@ func TestGitHubRepository_OnUpdate(t *testing.T) {
func TestGitHubRepository_OnDelete(t *testing.T) {
tests := []struct {
name string
setupMock func(m *github.MockClient)
setupMock func(m *github.MockClient, mockRepo *github.MockGithubRepository, mockSecrets *secrets.MockRepositorySecrets)
config *provisioning.Repository
webhookURL string
expectedError error
}{
{
name: "successfully delete webhook",
setupMock: func(m *github.MockClient) {
setupMock: func(m *github.MockClient, mockRepo *github.MockGithubRepository, mockSecrets *secrets.MockRepositorySecrets) {
mockRepo.On("OnDelete", mock.Anything).Return(nil)
mockSecrets.EXPECT().Delete(mock.Anything, mock.Anything, mock.Anything).Return(nil)
// Mock deleting the webhook
m.On("DeleteWebhook", mock.Anything, "grafana", "grafana", int64(123)).
Return(nil)
},
config: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
Branch: "main",
@@ -1717,19 +1722,32 @@ func TestGitHubRepository_OnDelete(t *testing.T) {
},
{
name: "no webhook URL provided",
setupMock: func(m *github.MockClient) {
// No mocks needed
setupMock: func(_ *github.MockClient, mockRepo *github.MockGithubRepository, _ *secrets.MockRepositorySecrets) {
mockRepo.On("OnDelete", mock.Anything).Return(nil)
},
config: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
Branch: "main",
},
},
},
config: &provisioning.Repository{},
webhookURL: "",
expectedError: nil,
},
{
name: "webhook not found in status",
setupMock: func(m *github.MockClient) {
// No mocks needed
setupMock: func(_ *github.MockClient, mockRepo *github.MockGithubRepository, _ *secrets.MockRepositorySecrets) {
mockRepo.On("OnDelete", mock.Anything).Return(nil)
// No secrets deletion or webhook deletion mocks needed - method returns early when webhook is nil
},
config: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
Branch: "main",
@@ -1740,16 +1758,45 @@ func TestGitHubRepository_OnDelete(t *testing.T) {
},
},
webhookURL: "https://example.com/webhook",
expectedError: fmt.Errorf("webhook not found"),
expectedError: nil, // No error expected - method returns early when webhook is nil
},
{
name: "error on delete from basic github repository",
setupMock: func(_ *github.MockClient, mockRepo *github.MockGithubRepository, _ *secrets.MockRepositorySecrets) {
mockRepo.On("OnDelete", mock.Anything).Return(fmt.Errorf("failed to delete webhook"))
},
config: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
Branch: "main",
},
},
Status: provisioning.RepositoryStatus{
Webhook: &provisioning.WebhookStatus{
ID: 123,
URL: "https://example.com/webhook",
},
},
},
webhookURL: "https://example.com/webhook",
expectedError: fmt.Errorf("on delete from basic github repository: failed to delete webhook"),
},
{
name: "error deleting webhook",
setupMock: func(m *github.MockClient) {
setupMock: func(m *github.MockClient, mockRepo *github.MockGithubRepository, mockSecrets *secrets.MockRepositorySecrets) {
mockRepo.On("OnDelete", mock.Anything).Return(nil)
mockSecrets.EXPECT().Delete(mock.Anything, mock.Anything, mock.Anything).Return(nil)
// Mock webhook deletion failure
m.On("DeleteWebhook", mock.Anything, "grafana", "grafana", int64(123)).
Return(fmt.Errorf("failed to delete webhook"))
},
config: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
Branch: "main",
@@ -1771,15 +1818,19 @@ func TestGitHubRepository_OnDelete(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
// Setup mock GitHub client
mockGH := github.NewMockClient(t)
tt.setupMock(mockGH)
mockRepo := github.NewMockGithubRepository(t)
mockSecrets := secrets.NewMockRepositorySecrets(t)
tt.setupMock(mockGH, mockRepo, mockSecrets)
// Create repository with mock
repo := &githubWebhookRepository{
gh: mockGH,
config: tt.config,
owner: "grafana",
repo: "grafana",
webhookURL: tt.webhookURL,
GithubRepository: mockRepo,
gh: mockGH,
config: tt.config,
secrets: mockSecrets,
owner: "grafana",
repo: "grafana",
webhookURL: tt.webhookURL,
}
// Call the OnDelete method
@@ -1798,3 +1849,156 @@ func TestGitHubRepository_OnDelete(t *testing.T) {
})
}
}
func TestGitHubRepository_OnDelete_WithSecrets(t *testing.T) {
tests := []struct {
name string
setupMock func(m *github.MockClient, mockRepo *github.MockGithubRepository, mockSecrets *secrets.MockRepositorySecrets)
config *provisioning.Repository
webhookURL string
expectedError string
}{
{
name: "successful deletion with secrets",
setupMock: func(m *github.MockClient, mockRepo *github.MockGithubRepository, mockSecrets *secrets.MockRepositorySecrets) {
mockRepo.On("OnDelete", mock.Anything).Return(nil)
mockSecrets.EXPECT().Delete(
mock.Anything,
&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
Branch: "main",
},
},
Status: provisioning.RepositoryStatus{
Webhook: &provisioning.WebhookStatus{
ID: 123,
URL: "https://example.com/webhook",
},
},
},
"test-repo"+webhookSecretSuffix,
).Return(nil)
m.On("DeleteWebhook", mock.Anything, "grafana", "grafana", int64(123)).Return(nil)
},
config: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
Branch: "main",
},
},
Status: provisioning.RepositoryStatus{
Webhook: &provisioning.WebhookStatus{
ID: 123,
URL: "https://example.com/webhook",
},
},
},
webhookURL: "https://example.com/webhook",
},
{
name: "secret deletion error",
setupMock: func(_ *github.MockClient, mockRepo *github.MockGithubRepository, mockSecrets *secrets.MockRepositorySecrets) {
mockRepo.On("OnDelete", mock.Anything).Return(nil)
mockSecrets.EXPECT().Delete(
mock.Anything,
&provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
Branch: "main",
},
},
Status: provisioning.RepositoryStatus{
Webhook: &provisioning.WebhookStatus{
ID: 123,
URL: "https://example.com/webhook",
},
},
},
"test-repo"+webhookSecretSuffix,
).Return(errors.New("failed to delete webhook secret"))
},
config: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
Branch: "main",
},
},
Status: provisioning.RepositoryStatus{
Webhook: &provisioning.WebhookStatus{
ID: 123,
URL: "https://example.com/webhook",
},
},
},
webhookURL: "https://example.com/webhook",
expectedError: "delete webhook secret: failed to delete webhook secret",
},
{
name: "no webhook URL - no secrets deletion",
setupMock: func(_ *github.MockClient, mockRepo *github.MockGithubRepository, _ *secrets.MockRepositorySecrets) {
mockRepo.On("OnDelete", mock.Anything).Return(nil)
},
config: &provisioning.Repository{
ObjectMeta: metav1.ObjectMeta{
Name: "test-repo",
Namespace: "default",
},
Spec: provisioning.RepositorySpec{
GitHub: &provisioning.GitHubRepositoryConfig{
Branch: "main",
},
},
},
webhookURL: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mockClient := github.NewMockClient(t)
mockRepo := github.NewMockGithubRepository(t)
mockSecrets := secrets.NewMockRepositorySecrets(t)
tt.setupMock(mockClient, mockRepo, mockSecrets)
repo := &githubWebhookRepository{
GithubRepository: mockRepo,
gh: mockClient,
config: tt.config,
secrets: mockSecrets,
owner: "grafana",
repo: "grafana",
webhookURL: tt.webhookURL,
}
err := repo.OnDelete(context.Background())
if tt.expectedError != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tt.expectedError)
} else {
require.NoError(t, err)
}
mockClient.AssertExpectations(t)
mockRepo.AssertExpectations(t)
mockSecrets.AssertExpectations(t)
})
}
}