diff --git a/.gitignore b/.gitignore index d6dd8fb6d83..fb7d5d30a9a 100644 --- a/.gitignore +++ b/.gitignore @@ -250,8 +250,6 @@ public/mockServiceWorker.js /e2e-playwright/test-plugins/*/dist /apps/provisioning/cmd/job-controller/bin/ + # Ignore unified storage kv store files /grafana-kv-data - -# Ignore debug output from test library -/pkg/storage/secret/metadata/testdata/rapid/TestStateMachine/ diff --git a/apps/provisioning/pkg/apis/provisioning/v0alpha1/jobs.go b/apps/provisioning/pkg/apis/provisioning/v0alpha1/jobs.go index 847da88fbc5..b96fb1a6d27 100644 --- a/apps/provisioning/pkg/apis/provisioning/v0alpha1/jobs.go +++ b/apps/provisioning/pkg/apis/provisioning/v0alpha1/jobs.go @@ -221,9 +221,9 @@ func (in JobStatus) ToSyncStatus(jobId string) SyncStatus { } type JobResourceSummary struct { - Group string `json:"group,omitempty"` - Resource string `json:"resource,omitempty"` - Total int64 `json:"total,omitempty"` // the count (if known) + Group string `json:"group,omitempty"` + Kind string `json:"kind,omitempty"` + Total int64 `json:"total,omitempty"` // the count (if known) Create int64 `json:"create,omitempty"` Update int64 `json:"update,omitempty"` diff --git a/apps/provisioning/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go b/apps/provisioning/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go index 74f7b7d165f..18c385ce59c 100644 --- a/apps/provisioning/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go +++ b/apps/provisioning/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go @@ -846,7 +846,7 @@ func schema_pkg_apis_provisioning_v0alpha1_JobResourceSummary(ref common.Referen Format: "", }, }, - "resource": { + "kind": { SchemaProps: spec.SchemaProps{ Type: []string{"string"}, Format: "", diff --git a/apps/provisioning/pkg/generated/applyconfiguration/provisioning/v0alpha1/jobresourcesummary.go b/apps/provisioning/pkg/generated/applyconfiguration/provisioning/v0alpha1/jobresourcesummary.go index 75b69663794..ed6a62f651a 100644 --- a/apps/provisioning/pkg/generated/applyconfiguration/provisioning/v0alpha1/jobresourcesummary.go +++ b/apps/provisioning/pkg/generated/applyconfiguration/provisioning/v0alpha1/jobresourcesummary.go @@ -7,16 +7,16 @@ package v0alpha1 // JobResourceSummaryApplyConfiguration represents a declarative configuration of the JobResourceSummary type for use // with apply. type JobResourceSummaryApplyConfiguration struct { - Group *string `json:"group,omitempty"` - Resource *string `json:"resource,omitempty"` - Total *int64 `json:"total,omitempty"` - Create *int64 `json:"create,omitempty"` - Update *int64 `json:"update,omitempty"` - Delete *int64 `json:"delete,omitempty"` - Write *int64 `json:"write,omitempty"` - Error *int64 `json:"error,omitempty"` - Noop *int64 `json:"noop,omitempty"` - Errors []string `json:"errors,omitempty"` + Group *string `json:"group,omitempty"` + Kind *string `json:"kind,omitempty"` + Total *int64 `json:"total,omitempty"` + Create *int64 `json:"create,omitempty"` + Update *int64 `json:"update,omitempty"` + Delete *int64 `json:"delete,omitempty"` + Write *int64 `json:"write,omitempty"` + Error *int64 `json:"error,omitempty"` + Noop *int64 `json:"noop,omitempty"` + Errors []string `json:"errors,omitempty"` } // JobResourceSummaryApplyConfiguration constructs a declarative configuration of the JobResourceSummary type for use with @@ -33,11 +33,11 @@ func (b *JobResourceSummaryApplyConfiguration) WithGroup(value string) *JobResou return b } -// WithResource sets the Resource field in the declarative configuration to the given value +// WithKind sets the Kind field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the Resource field is set to the value of the last call. -func (b *JobResourceSummaryApplyConfiguration) WithResource(value string) *JobResourceSummaryApplyConfiguration { - b.Resource = &value +// If called multiple times, the Kind field is set to the value of the last call. +func (b *JobResourceSummaryApplyConfiguration) WithKind(value string) *JobResourceSummaryApplyConfiguration { + b.Kind = &value return b } diff --git a/apps/provisioning/pkg/repository/github/testdata/webhook-push-keep_file_only.json b/apps/provisioning/pkg/repository/github/testdata/webhook-push-keep_file_only.json new file mode 100644 index 00000000000..f4ab95c170e --- /dev/null +++ b/apps/provisioning/pkg/repository/github/testdata/webhook-push-keep_file_only.json @@ -0,0 +1,105 @@ +{ + "ref": "refs/heads/main", + "before": "72096e3adc646c5a5b8a91744f962b12bac06045", + "after": "1234567890abcdef1234567890abcdef12345678", + "repository": { + "id": 888020043, + "node_id": "R_kgDONO4cSw", + "name": "git-ui-sync-demo", + "full_name": "grafana/git-ui-sync-demo", + "private": true, + "owner": { + "name": "grafana", + "email": "hello@grafana.com", + "login": "grafana", + "id": 7195757, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjcxOTU3NTc=", + "avatar_url": "https://avatars.githubusercontent.com/u/7195757?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/grafana", + "html_url": "https://github.com/grafana", + "type": "Organization", + "site_admin": false + }, + "html_url": "https://github.com/grafana/git-ui-sync-demo", + "description": "A repository containing Grafana dashboards to demo the Github Sync feature in Grafana.", + "fork": false, + "url": "https://github.com/grafana/git-ui-sync-demo", + "default_branch": "main", + "master_branch": "main", + "organization": "grafana" + }, + "pusher": { + "name": "testuser", + "email": "test@grafana.com" + }, + "organization": { + "login": "grafana", + "id": 7195757, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjcxOTU3NTc=", + "url": "https://api.github.com/orgs/grafana", + "avatar_url": "https://avatars.githubusercontent.com/u/7195757?v=4" + }, + "sender": { + "login": "testuser", + "id": 123456, + "node_id": "MDQ6VXNlcjEyMzQ1Ng==", + "avatar_url": "https://avatars.githubusercontent.com/u/123456?v=4", + "type": "User", + "site_admin": false + }, + "created": false, + "deleted": false, + "forced": false, + "base_ref": null, + "compare": "https://github.com/grafana/git-ui-sync-demo/compare/72096e3adc64...1234567890ab", + "commits": [ + { + "id": "1234567890abcdef1234567890abcdef12345678", + "tree_id": "abcdef1234567890abcdef1234567890abcdef12", + "distinct": true, + "message": "Remove empty folder by deleting .keep file", + "timestamp": "2024-12-09T11:00:48+03:00", + "url": "https://github.com/grafana/git-ui-sync-demo/commit/1234567890abcdef1234567890abcdef12345678", + "author": { + "name": "Test User", + "email": "test@grafana.com", + "username": "testuser" + }, + "committer": { + "name": "Test User", + "email": "test@grafana.com", + "username": "testuser" + }, + "added": [], + "removed": [ + "empty-folder/.keep" + ], + "modified": [] + } + ], + "head_commit": { + "id": "1234567890abcdef1234567890abcdef12345678", + "tree_id": "abcdef1234567890abcdef1234567890abcdef12", + "distinct": true, + "message": "Remove empty folder by deleting .keep file", + "timestamp": "2024-12-09T11:00:48+03:00", + "url": "https://github.com/grafana/git-ui-sync-demo/commit/1234567890abcdef1234567890abcdef12345678", + "author": { + "name": "Test User", + "email": "test@grafana.com", + "username": "testuser" + }, + "committer": { + "name": "Test User", + "email": "test@grafana.com", + "username": "testuser" + }, + "added": [], + "removed": [ + "empty-folder/.keep" + ], + "modified": [] + } +} + diff --git a/apps/provisioning/pkg/repository/github/testdata/webhook-push-keep_file_with_others.json b/apps/provisioning/pkg/repository/github/testdata/webhook-push-keep_file_with_others.json new file mode 100644 index 00000000000..9a70ca4997c --- /dev/null +++ b/apps/provisioning/pkg/repository/github/testdata/webhook-push-keep_file_with_others.json @@ -0,0 +1,109 @@ +{ + "ref": "refs/heads/main", + "before": "72096e3adc646c5a5b8a91744f962b12bac06045", + "after": "2345678901bcdef2345678901bcdef2345678901", + "repository": { + "id": 888020043, + "node_id": "R_kgDONO4cSw", + "name": "git-ui-sync-demo", + "full_name": "grafana/git-ui-sync-demo", + "private": true, + "owner": { + "name": "grafana", + "email": "hello@grafana.com", + "login": "grafana", + "id": 7195757, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjcxOTU3NTc=", + "avatar_url": "https://avatars.githubusercontent.com/u/7195757?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/grafana", + "html_url": "https://github.com/grafana", + "type": "Organization", + "site_admin": false + }, + "html_url": "https://github.com/grafana/git-ui-sync-demo", + "description": "A repository containing Grafana dashboards to demo the Github Sync feature in Grafana.", + "fork": false, + "url": "https://github.com/grafana/git-ui-sync-demo", + "default_branch": "main", + "master_branch": "main", + "organization": "grafana" + }, + "pusher": { + "name": "testuser", + "email": "test@grafana.com" + }, + "organization": { + "login": "grafana", + "id": 7195757, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjcxOTU3NTc=", + "url": "https://api.github.com/orgs/grafana", + "avatar_url": "https://avatars.githubusercontent.com/u/7195757?v=4" + }, + "sender": { + "login": "testuser", + "id": 123456, + "node_id": "MDQ6VXNlcjEyMzQ1Ng==", + "avatar_url": "https://avatars.githubusercontent.com/u/123456?v=4", + "type": "User", + "site_admin": false + }, + "created": false, + "deleted": false, + "forced": false, + "base_ref": null, + "compare": "https://github.com/grafana/git-ui-sync-demo/compare/72096e3adc64...2345678901bc", + "commits": [ + { + "id": "2345678901bcdef2345678901bcdef2345678901", + "tree_id": "bcdef2345678901bcdef2345678901bcdef23456", + "distinct": true, + "message": "Remove folder with .keep and dashboard files", + "timestamp": "2024-12-09T11:00:48+03:00", + "url": "https://github.com/grafana/git-ui-sync-demo/commit/2345678901bcdef2345678901bcdef2345678901", + "author": { + "name": "Test User", + "email": "test@grafana.com", + "username": "testuser" + }, + "committer": { + "name": "Test User", + "email": "test@grafana.com", + "username": "testuser" + }, + "added": [], + "removed": [ + "dashboards/.keep", + "dashboards/dashboard1.json", + "dashboards/dashboard2.json" + ], + "modified": [] + } + ], + "head_commit": { + "id": "2345678901bcdef2345678901bcdef2345678901", + "tree_id": "bcdef2345678901bcdef2345678901bcdef23456", + "distinct": true, + "message": "Remove folder with .keep and dashboard files", + "timestamp": "2024-12-09T11:00:48+03:00", + "url": "https://github.com/grafana/git-ui-sync-demo/commit/2345678901bcdef2345678901bcdef2345678901", + "author": { + "name": "Test User", + "email": "test@grafana.com", + "username": "testuser" + }, + "committer": { + "name": "Test User", + "email": "test@grafana.com", + "username": "testuser" + }, + "added": [], + "removed": [ + "dashboards/.keep", + "dashboards/dashboard1.json", + "dashboards/dashboard2.json" + ], + "modified": [] + } +} + diff --git a/apps/provisioning/pkg/repository/github/testdata/webhook-push-multiple_keep_files.json b/apps/provisioning/pkg/repository/github/testdata/webhook-push-multiple_keep_files.json new file mode 100644 index 00000000000..cbb82242aa3 --- /dev/null +++ b/apps/provisioning/pkg/repository/github/testdata/webhook-push-multiple_keep_files.json @@ -0,0 +1,109 @@ +{ + "ref": "refs/heads/main", + "before": "72096e3adc646c5a5b8a91744f962b12bac06045", + "after": "3456789012cdef3456789012cdef3456789012cd", + "repository": { + "id": 888020043, + "node_id": "R_kgDONO4cSw", + "name": "git-ui-sync-demo", + "full_name": "grafana/git-ui-sync-demo", + "private": true, + "owner": { + "name": "grafana", + "email": "hello@grafana.com", + "login": "grafana", + "id": 7195757, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjcxOTU3NTc=", + "avatar_url": "https://avatars.githubusercontent.com/u/7195757?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/grafana", + "html_url": "https://github.com/grafana", + "type": "Organization", + "site_admin": false + }, + "html_url": "https://github.com/grafana/git-ui-sync-demo", + "description": "A repository containing Grafana dashboards to demo the Github Sync feature in Grafana.", + "fork": false, + "url": "https://github.com/grafana/git-ui-sync-demo", + "default_branch": "main", + "master_branch": "main", + "organization": "grafana" + }, + "pusher": { + "name": "testuser", + "email": "test@grafana.com" + }, + "organization": { + "login": "grafana", + "id": 7195757, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjcxOTU3NTc=", + "url": "https://api.github.com/orgs/grafana", + "avatar_url": "https://avatars.githubusercontent.com/u/7195757?v=4" + }, + "sender": { + "login": "testuser", + "id": 123456, + "node_id": "MDQ6VXNlcjEyMzQ1Ng==", + "avatar_url": "https://avatars.githubusercontent.com/u/123456?v=4", + "type": "User", + "site_admin": false + }, + "created": false, + "deleted": false, + "forced": false, + "base_ref": null, + "compare": "https://github.com/grafana/git-ui-sync-demo/compare/72096e3adc64...3456789012cd", + "commits": [ + { + "id": "3456789012cdef3456789012cdef3456789012cd", + "tree_id": "cdef3456789012cdef3456789012cdef34567890", + "distinct": true, + "message": "Remove multiple folders, some with only .keep files", + "timestamp": "2024-12-09T11:00:48+03:00", + "url": "https://github.com/grafana/git-ui-sync-demo/commit/3456789012cdef3456789012cdef3456789012cd", + "author": { + "name": "Test User", + "email": "test@grafana.com", + "username": "testuser" + }, + "committer": { + "name": "Test User", + "email": "test@grafana.com", + "username": "testuser" + }, + "added": [], + "removed": [ + "empty-folder1/.keep", + "dashboards-to-delete/.keep", + "dashboards-to-delete/dashboard.json" + ], + "modified": [] + } + ], + "head_commit": { + "id": "3456789012cdef3456789012cdef3456789012cd", + "tree_id": "cdef3456789012cdef3456789012cdef34567890", + "distinct": true, + "message": "Remove multiple folders, some with only .keep files", + "timestamp": "2024-12-09T11:00:48+03:00", + "url": "https://github.com/grafana/git-ui-sync-demo/commit/3456789012cdef3456789012cdef3456789012cd", + "author": { + "name": "Test User", + "email": "test@grafana.com", + "username": "testuser" + }, + "committer": { + "name": "Test User", + "email": "test@grafana.com", + "username": "testuser" + }, + "added": [], + "removed": [ + "empty-folder1/.keep", + "dashboards-to-delete/.keep", + "dashboards-to-delete/dashboard.json" + ], + "modified": [] + } +} + diff --git a/apps/provisioning/pkg/repository/github/webhook.go b/apps/provisioning/pkg/repository/github/webhook.go index c2e13681f33..40f87ddc93d 100644 --- a/apps/provisioning/pkg/repository/github/webhook.go +++ b/apps/provisioning/pkg/repository/github/webhook.go @@ -7,6 +7,7 @@ import ( "log/slog" "net/http" "slices" + "strings" "github.com/google/go-github/v70/github" "github.com/google/uuid" @@ -15,6 +16,7 @@ import ( "github.com/grafana/grafana-app-sdk/logging" 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/safepath" common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1" ) @@ -119,13 +121,38 @@ func (r *githubWebhookRepository) parsePushEvent(event *github.PushEvent) (*prov return &provisioning.WebhookResponse{Code: http.StatusOK}, nil } + // whenever possible, we want to do incremental syncs to keep things performant. + // however, if we get an event where just a .keep file is being deleted, and no other files in the folder + // are being deleted, the folder could be gone from git, but not from grafana and we do not have a way + // to get the grafana uid to delete the folder. so, instead, we will queue a full sync to clean things up. + dirsWithKeepDeletes := make(map[string]struct{}) + dirsWithOtherDeletes := make(map[string]struct{}) + for _, change := range event.GetCommits() { + for _, removedFile := range change.Removed { + dir := safepath.Dir(removedFile) + if strings.HasSuffix(removedFile, ".keep") { + dirsWithKeepDeletes[dir] = struct{}{} + } else { + dirsWithOtherDeletes[dir] = struct{}{} + } + } + } + // if there are any keep files deleted that do not have other files deleted in the same folder, we need to queue a full sync + incremental := true + for dir := range dirsWithKeepDeletes { + if _, exists := dirsWithOtherDeletes[dir]; !exists { + incremental = false + break + } + } + return &provisioning.WebhookResponse{ Code: http.StatusAccepted, Job: &provisioning.JobSpec{ Repository: r.config.GetName(), Action: provisioning.JobActionPull, Pull: &provisioning.SyncJobOptions{ - Incremental: true, + Incremental: incremental, }, }, }, nil diff --git a/apps/provisioning/pkg/repository/github/webhook_test.go b/apps/provisioning/pkg/repository/github/webhook_test.go index 49faf733378..5dc23c70e34 100644 --- a/apps/provisioning/pkg/repository/github/webhook_test.go +++ b/apps/provisioning/pkg/repository/github/webhook_test.go @@ -69,6 +69,36 @@ func TestParseWebhooks(t *testing.T) { }, }, }}, + {"push", "keep_file_only", provisioning.WebhookResponse{ + Code: http.StatusAccepted, + Job: &provisioning.JobSpec{ + Repository: "unit-test-repo", + Action: provisioning.JobActionPull, + Pull: &provisioning.SyncJobOptions{ + Incremental: false, + }, + }, + }}, + {"push", "keep_file_with_others", provisioning.WebhookResponse{ + Code: http.StatusAccepted, + Job: &provisioning.JobSpec{ + Repository: "unit-test-repo", + Action: provisioning.JobActionPull, + Pull: &provisioning.SyncJobOptions{ + Incremental: true, + }, + }, + }}, + {"push", "multiple_keep_files", provisioning.WebhookResponse{ + Code: http.StatusAccepted, + Job: &provisioning.JobSpec{ + Repository: "unit-test-repo", + Action: provisioning.JobActionPull, + Pull: &provisioning.SyncJobOptions{ + Incremental: false, + }, + }, + }}, {"issue_comment", "created", provisioning.WebhookResponse{ Code: http.StatusNotImplemented, }}, diff --git a/pkg/registry/apis/dashboard/register.go b/pkg/registry/apis/dashboard/register.go index adeb3d7b1a5..a8e5de42e39 100644 --- a/pkg/registry/apis/dashboard/register.go +++ b/pkg/registry/apis/dashboard/register.go @@ -651,7 +651,7 @@ func (b *DashboardsAPIBuilder) afterDelete(obj runtime.Object, _ *metav1.DeleteO client := (*b.resourcePermissionsSvc).Namespace(meta.GetNamespace()) name := fmt.Sprintf("%s-%s-%s", dashv1.DashboardResourceInfo.GroupVersionResource().Group, dashv1.DashboardResourceInfo.GroupVersionResource().Resource, meta.GetName()) err = client.Delete(ctx, name, metav1.DeleteOptions{}) - if err != nil { + if err != nil && !apierrors.IsNotFound(err) { log.Error("failed to delete dashboard permissions", "error", err) } } diff --git a/pkg/registry/apis/folders/hooks.go b/pkg/registry/apis/folders/hooks.go index 9adf40c1293..bf8f042769f 100644 --- a/pkg/registry/apis/folders/hooks.go +++ b/pkg/registry/apis/folders/hooks.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/generic/registry" @@ -94,7 +95,7 @@ func (b *FolderAPIBuilder) afterDelete(obj runtime.Object, _ *metav1.DeleteOptio log.Debug("deleting folder permissions", "uid", meta.GetName(), "namespace", meta.GetNamespace()) client := (*b.resourcePermissionsSvc).Namespace(meta.GetNamespace()) err := client.Delete(ctx, fmt.Sprintf("%s-%s-%s", folders.FolderResourceInfo.GroupVersionResource().Group, folders.FolderResourceInfo.GroupVersionResource().Resource, meta.GetName()), metav1.DeleteOptions{}) - if err != nil { + if err != nil && !apierrors.IsNotFound(err) { log.Error("failed to delete folder permissions", "error", err) } return diff --git a/pkg/registry/apis/provisioning/jobs/export/folders.go b/pkg/registry/apis/provisioning/jobs/export/folders.go index cb3b4a5f059..4b11f03172a 100644 --- a/pkg/registry/apis/provisioning/jobs/export/folders.go +++ b/pkg/registry/apis/provisioning/jobs/export/folders.go @@ -45,11 +45,11 @@ func ExportFolders(ctx context.Context, repoName string, options provisioning.Ex progress.SetMessage(ctx, "write folders to repository") err := repositoryResources.EnsureFolderTreeExists(ctx, options.Branch, options.Path, tree, func(folder resources.Folder, created bool, err error) error { result := jobs.JobResourceResult{ - Action: repository.FileActionCreated, - Name: folder.ID, - Resource: resources.FolderResource.Resource, - Group: resources.FolderResource.Group, - Path: folder.Path, + Action: repository.FileActionCreated, + Name: folder.ID, + Group: resources.FolderResource.Group, + Kind: resources.FolderKind.Kind, + Path: folder.Path, } if err != nil { diff --git a/pkg/registry/apis/provisioning/jobs/export/resources.go b/pkg/registry/apis/provisioning/jobs/export/resources.go index c07a871815a..445f25faca4 100644 --- a/pkg/registry/apis/provisioning/jobs/export/resources.go +++ b/pkg/registry/apis/provisioning/jobs/export/resources.go @@ -98,10 +98,10 @@ func exportResource(ctx context.Context, return resources.ForEach(ctx, client, func(item *unstructured.Unstructured) (err error) { gvk := item.GroupVersionKind() result := jobs.JobResourceResult{ - Name: item.GetName(), - Resource: resource, - Group: gvk.Group, - Action: repository.FileActionCreated, + Name: item.GetName(), + Group: gvk.Group, + Kind: gvk.Kind, + Action: repository.FileActionCreated, } // Check if resource is already managed by a repository diff --git a/pkg/registry/apis/provisioning/jobs/loki_history_test.go b/pkg/registry/apis/provisioning/jobs/loki_history_test.go index 026e889431d..26d7f01ab4f 100644 --- a/pkg/registry/apis/provisioning/jobs/loki_history_test.go +++ b/pkg/registry/apis/provisioning/jobs/loki_history_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - "github.com/grafana/grafana/apps/provisioning/pkg/loki" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -17,6 +16,7 @@ import ( "k8s.io/apimachinery/pkg/types" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/apps/provisioning/pkg/loki" ) func TestLokiJobHistory_WriteJob(t *testing.T) { @@ -86,16 +86,16 @@ func TestLokiJobHistory_WriteJob(t *testing.T) { Errors: []string{"warning: deprecated field used"}, Progress: 100.0, Summary: []*provisioning.JobResourceSummary{{ - Group: "dashboard.grafana.app", - Resource: "dashboards", - Total: 10, - Create: 3, - Update: 5, - Delete: 1, - Write: 8, - Error: 1, - Noop: 0, - Errors: []string{"failed to process dashboard-x"}, + Group: "dashboard.grafana.app", + Kind: "Dashboard", + Total: 10, + Create: 3, + Update: 5, + Delete: 1, + Write: 8, + Error: 1, + Noop: 0, + Errors: []string{"failed to process dashboard-x"}, }}, }, } @@ -178,7 +178,7 @@ func TestLokiJobHistory_WriteJob(t *testing.T) { require.Len(t, deserializedJob.Status.Summary, 1) summary := deserializedJob.Status.Summary[0] assert.Equal(t, "dashboard.grafana.app", summary.Group) - assert.Equal(t, "dashboards", summary.Resource) + assert.Equal(t, "Dashboard", summary.Kind) assert.Equal(t, int64(10), summary.Total) assert.Equal(t, int64(3), summary.Create) assert.Equal(t, int64(5), summary.Update) diff --git a/pkg/registry/apis/provisioning/jobs/migrate/clean.go b/pkg/registry/apis/provisioning/jobs/migrate/clean.go index c1c606db71d..c82bbdaea2f 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/clean.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/clean.go @@ -4,12 +4,13 @@ import ( "context" "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/grafana/grafana/apps/provisioning/pkg/repository" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) //go:generate mockery --name NamespaceCleaner --structname MockNamespaceCleaner --inpackage --filename mock_namespace_cleaner.go --with-expecter @@ -40,10 +41,10 @@ func (c *namespaceCleaner) Clean(ctx context.Context, namespace string, progress if err = resources.ForEach(ctx, client, func(item *unstructured.Unstructured) error { result := jobs.JobResourceResult{ - Name: item.GetName(), - Resource: item.GetKind(), - Group: item.GroupVersionKind().Group, - Action: repository.FileActionDeleted, + Name: item.GetName(), + Kind: item.GetKind(), + Group: item.GroupVersionKind().Group, + Action: repository.FileActionDeleted, } // Skip provisioned resources - only delete unprovisioned (unmanaged) resources @@ -63,7 +64,7 @@ func (c *namespaceCleaner) Clean(ctx context.Context, namespace string, progress } if err := client.Delete(ctx, item.GetName(), metav1.DeleteOptions{}); err != nil { - result.Error = fmt.Errorf("deleting resource %s/%s %s: %w", result.Group, result.Resource, result.Name, err) + result.Error = fmt.Errorf("deleting resource %s/%s %s: %w", result.Group, result.Kind, result.Name, err) progress.Record(ctx, result) return fmt.Errorf("delete resource: %w", err) } diff --git a/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources.go index 5252945cad9..b7b4bb45a4f 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources.go @@ -204,11 +204,11 @@ func (r *legacyResourceResourceMigrator) Write(ctx context.Context, key *resourc } result := jobs.JobResourceResult{ - Name: parsed.Meta.GetName(), - Resource: r.kind.Resource, - Group: r.kind.Group, - Action: repository.FileActionCreated, - Path: fileName, + Name: parsed.Meta.GetName(), + Group: r.kind.Group, + Kind: parsed.GVK.Kind, + Action: repository.FileActionCreated, + Path: fileName, } if err != nil { diff --git a/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources_test.go b/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources_test.go index 3fc6a01e011..89077848f25 100644 --- a/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources_test.go +++ b/pkg/registry/apis/provisioning/jobs/migrate/legacy_resources_test.go @@ -667,7 +667,7 @@ func TestLegacyResourceResourceMigrator_Write(t *testing.T) { return result.Action == repository.FileActionCreated && result.Name == "test" && result.Error == nil && - result.Resource == "tests" && + result.Kind == "" && // empty kind result.Group == "test.grafana.app" && result.Path == "test/path" })).Return() diff --git a/pkg/registry/apis/provisioning/jobs/progress.go b/pkg/registry/apis/provisioning/jobs/progress.go index 3286ce37a29..f9cea399865 100644 --- a/pkg/registry/apis/provisioning/jobs/progress.go +++ b/pkg/registry/apis/provisioning/jobs/progress.go @@ -35,12 +35,12 @@ func maybeNotifyProgress(threshold time.Duration, fn ProgressFn) ProgressFn { // FIXME: ProgressRecorder should be initialized in the queue type JobResourceResult struct { - Name string - Resource string - Group string - Path string - Action repository.FileAction - Error error + Name string + Group string + Kind string + Path string + Action repository.FileAction + Error error } type jobProgressRecorder struct { @@ -73,7 +73,7 @@ func (r *jobProgressRecorder) Record(ctx context.Context, result JobResourceResu r.mu.Lock() r.resultCount++ - logger := logging.FromContext(ctx).With("path", result.Path, "resource", result.Resource, "group", result.Group, "action", result.Action, "name", result.Name) + logger := logging.FromContext(ctx).With("path", result.Path, "group", result.Group, "kind", result.Kind, "action", result.Action, "name", result.Name) if result.Error != nil { logger.Error("job resource operation failed", "err", result.Error) if len(r.errors) < 20 { @@ -173,12 +173,12 @@ func (r *jobProgressRecorder) summary() []*provisioning.JobResourceSummary { func (r *jobProgressRecorder) updateSummary(result JobResourceResult) { // Note: This method is called from Record() which already holds the lock - key := result.Resource + ":" + result.Group + key := result.Group + ":" + result.Kind summary, exists := r.summaries[key] if !exists { summary = &provisioning.JobResourceSummary{ - Resource: result.Resource, - Group: result.Group, + Group: result.Group, + Kind: result.Kind, } r.summaries[key] = summary } diff --git a/pkg/registry/apis/provisioning/jobs/sync/full.go b/pkg/registry/apis/provisioning/jobs/sync/full.go index 0e0b107171a..075f2fbac5f 100644 --- a/pkg/registry/apis/provisioning/jobs/sync/full.go +++ b/pkg/registry/apis/provisioning/jobs/sync/full.go @@ -4,15 +4,16 @@ import ( "context" "fmt" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/grafana/grafana/apps/provisioning/pkg/repository" "github.com/grafana/grafana/apps/provisioning/pkg/safepath" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/trace" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" ) func FullSync( @@ -92,9 +93,7 @@ func applyChanges(ctx context.Context, changes []ResourceFileChange, clients res deleteSpan.End() continue } - result.Name = change.Existing.Name - result.Resource = change.Existing.Resource result.Group = change.Existing.Group versionlessGVR := schema.GroupVersionResource{ @@ -103,15 +102,17 @@ func applyChanges(ctx context.Context, changes []ResourceFileChange, clients res } // TODO: should we use the clients or the resource manager instead? - client, _, err := clients.ForResource(deleteCtx, versionlessGVR) + client, gvk, err := clients.ForResource(deleteCtx, versionlessGVR) if err != nil { + result.Kind = versionlessGVR.Resource // could not find a kind result.Error = fmt.Errorf("get client for deleted object: %w", err) progress.Record(deleteCtx, result) continue } + result.Kind = gvk.Kind if err := client.Delete(deleteCtx, change.Existing.Name, metav1.DeleteOptions{}); err != nil { - result.Error = fmt.Errorf("deleting resource %s/%s %s: %w", change.Existing.Group, change.Existing.Resource, change.Existing.Name, err) + result.Error = fmt.Errorf("deleting resource %s/%s %s: %w", change.Existing.Group, gvk.Kind, change.Existing.Name, err) } progress.Record(deleteCtx, result) deleteSpan.End() @@ -122,10 +123,10 @@ func applyChanges(ctx context.Context, changes []ResourceFileChange, clients res if safepath.IsDir(change.Path) { ensureFolderCtx, ensureFolderSpan := tracer.Start(ctx, "provisioning.sync.full.apply_changes.ensure_folder_exists") result := jobs.JobResourceResult{ - Path: change.Path, - Action: change.Action, - Resource: resources.FolderResource.Resource, - Group: resources.FolderResource.Group, + Path: change.Path, + Action: change.Action, + Group: resources.FolderKind.Group, + Kind: resources.FolderKind.Kind, } folder, err := repositoryResources.EnsureFolderPathExist(ensureFolderCtx, change.Path) @@ -147,11 +148,11 @@ func applyChanges(ctx context.Context, changes []ResourceFileChange, clients res writeCtx, writeSpan := tracer.Start(ctx, "provisioning.sync.full.apply_changes.write_resource_from_file") name, gvk, err := repositoryResources.WriteResourceFromFile(writeCtx, change.Path, "") result := jobs.JobResourceResult{ - Path: change.Path, - Action: change.Action, - Name: name, - Resource: gvk.Kind, - Group: gvk.Group, + Path: change.Path, + Action: change.Action, + Name: name, + Group: gvk.Group, + Kind: gvk.Kind, } if err != nil { diff --git a/pkg/registry/apis/provisioning/jobs/sync/full_test.go b/pkg/registry/apis/provisioning/jobs/sync/full_test.go index d5b3b610b80..90e5f4fb0c1 100644 --- a/pkg/registry/apis/provisioning/jobs/sync/full_test.go +++ b/pkg/registry/apis/provisioning/jobs/sync/full_test.go @@ -6,11 +6,6 @@ import ( "fmt" "testing" - provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" - "github.com/grafana/grafana/apps/provisioning/pkg/repository" - "github.com/grafana/grafana/pkg/infra/tracing" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" - "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -18,6 +13,12 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" dynamicfake "k8s.io/client-go/dynamic/fake" k8testing "k8s.io/client-go/testing" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/apps/provisioning/pkg/repository" + "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" ) func TestFullSync_ContextCancelled(t *testing.T) { @@ -209,11 +210,11 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo Return("test-dashboard", schema.GroupVersionKind{Kind: "Dashboard", Group: "dashboards"}, nil) progress.On("Record", mock.Anything, jobs.JobResourceResult{ - Action: repository.FileActionCreated, - Path: "dashboards/one.json", - Name: "test-dashboard", - Resource: "Dashboard", - Group: "dashboards", + Action: repository.FileActionCreated, + Path: "dashboards/one.json", + Name: "test-dashboard", + Kind: "Dashboard", + Group: "dashboards", }).Return() }, expectedError: "too many errors", @@ -234,11 +235,11 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo Return("test-dashboard", schema.GroupVersionKind{Kind: "Dashboard", Group: "dashboards"}, nil) progress.On("Record", mock.Anything, jobs.JobResourceResult{ - Action: repository.FileActionCreated, - Path: "dashboards/test.json", - Name: "test-dashboard", - Resource: "Dashboard", - Group: "dashboards", + Action: repository.FileActionCreated, + Path: "dashboards/test.json", + Name: "test-dashboard", + Kind: "Dashboard", + Group: "dashboards", }).Return() }, }, @@ -261,7 +262,7 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo return result.Action == repository.FileActionCreated && result.Path == "dashboards/test.json" && result.Name == "test-dashboard" && - result.Resource == "Dashboard" && + result.Kind == "Dashboard" && result.Group == "dashboards" && result.Error != nil && result.Error.Error() == "writing resource from file dashboards/test.json: write error" @@ -284,11 +285,11 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo Return("test-dashboard", schema.GroupVersionKind{Kind: "Dashboard", Group: "dashboards"}, nil) progress.On("Record", mock.Anything, jobs.JobResourceResult{ - Action: repository.FileActionUpdated, - Path: "dashboards/test.json", - Name: "test-dashboard", - Resource: "Dashboard", - Group: "dashboards", + Action: repository.FileActionUpdated, + Path: "dashboards/test.json", + Name: "test-dashboard", + Kind: "Dashboard", + Group: "dashboards", }).Return() }, }, @@ -311,7 +312,7 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo return result.Action == repository.FileActionUpdated && result.Path == "dashboards/test.json" && result.Name == "test-dashboard" && - result.Resource == "Dashboard" && + result.Kind == "Dashboard" && result.Group == "dashboards" && result.Error != nil && result.Error.Error() == "writing resource from file dashboards/test.json: write error" @@ -335,9 +336,8 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo Action: repository.FileActionCreated, Path: "one/two/three/", Name: "some-folder", - // FIXME: this is probably inconsistent across the codebase - Resource: "folders", - Group: "folder.grafana.app", + Kind: "Folder", + Group: "folder.grafana.app", }).Return() }, }, @@ -362,7 +362,7 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo return result.Action == repository.FileActionCreated && result.Path == "one/two/three/" && result.Name == "" && - result.Resource == "folders" && + result.Kind == "Folder" && result.Group == "folder.grafana.app" && result.Error != nil && result.Error.Error() == "ensuring folder exists at path one/two/three/: folder creation error" @@ -378,7 +378,7 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo Path: "dashboards/test.json", Existing: &provisioning.ResourceListItem{ Name: "test-dashboard", - Resource: "Dashboard", + Resource: "dashboards", Group: "dashboards", }, }, @@ -411,7 +411,7 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo clients.On("ForResource", mock.Anything, schema.GroupVersionResource{ Group: "dashboards", - Resource: "Dashboard", + Resource: "dashboards", }).Return(fakeDynamicClient.Resource(resources.DashboardResource), schema.GroupVersionKind{ Kind: "Dashboard", Group: "dashboards", @@ -419,12 +419,12 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo }, nil) progress.On("Record", mock.Anything, jobs.JobResourceResult{ - Action: repository.FileActionDeleted, - Path: "dashboards/test.json", - Name: "test-dashboard", - Resource: "Dashboard", - Group: "dashboards", - Error: nil, + Action: repository.FileActionDeleted, + Path: "dashboards/test.json", + Name: "test-dashboard", + Kind: "Dashboard", + Group: "dashboards", + Error: nil, }).Return() }, }, @@ -437,7 +437,7 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo Path: "dashboards/test.json", Existing: &provisioning.ResourceListItem{ Name: "test-dashboard", - Resource: "Dashboard", + Resource: "dashboards", Group: "dashboards", }, }, @@ -470,7 +470,7 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo clients.On("ForResource", mock.Anything, schema.GroupVersionResource{ Group: "dashboards", - Resource: "Dashboard", + Resource: "dashboards", }).Return(fakeDynamicClient.Resource(resources.DashboardResource), schema.GroupVersionKind{ Kind: "Dashboard", Group: "dashboards", @@ -481,7 +481,7 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo return result.Action == repository.FileActionDeleted && result.Path == "dashboards/test.json" && result.Name == "test-dashboard" && - result.Resource == "Dashboard" && + result.Kind == "Dashboard" && result.Group == "dashboards" && result.Error != nil && result.Error.Error() == "deleting resource dashboards/Dashboard test-dashboard: delete failed" @@ -538,7 +538,7 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo Existing: &provisioning.ResourceListItem{ Name: "test-dashboard", Group: "dashboards", - Resource: "Dashboard", + Resource: "dashboards", }, }, }, @@ -547,16 +547,16 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo clients.On("ForResource", mock.Anything, schema.GroupVersionResource{ Group: "dashboards", - Resource: "Dashboard", + Resource: "dashboards", }).Return(nil, schema.GroupVersionKind{}, errors.New("didn't work")) progress.On("Record", mock.Anything, jobs.JobResourceResult{ - Name: "test-dashboard", - Group: "dashboards", - Resource: "Dashboard", - Action: repository.FileActionDeleted, - Path: "dashboards/test.json", - Error: fmt.Errorf("get client for deleted object: %w", errors.New("didn't work")), + Name: "test-dashboard", + Group: "dashboards", + Kind: "dashboards", // could not find a real kind + Action: repository.FileActionDeleted, + Path: "dashboards/test.json", + Error: fmt.Errorf("get client for deleted object: %w", errors.New("didn't work")), }).Return() }, }, @@ -610,12 +610,12 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo }, nil) progress.On("Record", mock.Anything, jobs.JobResourceResult{ - Action: repository.FileActionDeleted, - Path: "to-be-deleted/", - Name: "test-folder", - Resource: "Folder", - Group: "folders", - Error: nil, + Action: repository.FileActionDeleted, + Path: "to-be-deleted/", + Name: "test-folder", + Kind: "Folder", + Group: "folders", + Error: nil, }).Return() }, }, @@ -672,7 +672,7 @@ func TestFullSync_ApplyChanges(t *testing.T) { //nolint:gocyclo return result.Action == repository.FileActionDeleted && result.Path == "to-be-deleted/" && result.Name == "test-folder" && - result.Resource == "Folder" && + result.Kind == "Folder" && result.Group == "folders" && result.Error != nil && result.Error.Error() == "deleting resource folders/Folder test-folder: delete failed" diff --git a/pkg/registry/apis/provisioning/jobs/sync/incremental.go b/pkg/registry/apis/provisioning/jobs/sync/incremental.go index f7448bffa16..0c4ddde659b 100644 --- a/pkg/registry/apis/provisioning/jobs/sync/incremental.go +++ b/pkg/registry/apis/provisioning/jobs/sync/incremental.go @@ -2,6 +2,7 @@ package sync import ( "context" + "errors" "fmt" "github.com/grafana/grafana/apps/provisioning/pkg/repository" @@ -9,6 +10,9 @@ import ( "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" + apierrors "k8s.io/apimachinery/pkg/api/errors" ) // Convert git changes into resource file changes @@ -38,6 +42,11 @@ func IncrementalSync(ctx context.Context, repo repository.Versioned, previousRef progress.SetTotal(ctx, len(diff)) progress.SetMessage(ctx, "replicating versioned changes") + // this will keep track of any folders that had resources deleted from it + // with key-value as path:grafana uid. + // after cleaning up all resources, we will look to see if the foldrs are + // now empty, and if so, delete them. + affectedFolders := make(map[string]string) for _, change := range diff { if ctx.Err() != nil { return ctx.Err() @@ -63,11 +72,11 @@ func IncrementalSync(ctx context.Context, repo repository.Versioned, previousRef } progress.Record(ensureFolderCtx, jobs.JobResourceResult{ - Path: safeSegment, - Action: repository.FileActionCreated, - Resource: resources.FolderResource.Resource, - Group: resources.FolderResource.Group, - Name: folder, + Path: safeSegment, + Action: repository.FileActionCreated, + Group: resources.FolderResource.Group, + Kind: resources.FolderKind.Kind, + Name: folder, }) ensureFolderSpan.End() continue @@ -95,30 +104,40 @@ func IncrementalSync(ctx context.Context, repo repository.Versioned, previousRef result.Error = fmt.Errorf("writing resource from file %s: %w", change.Path, err) } result.Name = name - result.Resource = gvk.Kind + result.Kind = gvk.Kind result.Group = gvk.Group writeSpan.End() case repository.FileActionDeleted: removeCtx, removeSpan := tracer.Start(ctx, "provisioning.sync.incremental.remove_resource_from_file") - name, gvk, err := repositoryResources.RemoveResourceFromFile(removeCtx, change.Path, change.PreviousRef) + name, folderName, gvk, err := repositoryResources.RemoveResourceFromFile(removeCtx, change.Path, change.PreviousRef) if err != nil { removeSpan.RecordError(err) result.Error = fmt.Errorf("removing resource from file %s: %w", change.Path, err) } result.Name = name - result.Resource = gvk.Kind + result.Kind = gvk.Kind result.Group = gvk.Group + + if folderName != "" { + affectedFolders[safepath.Dir(change.Path)] = folderName + } + removeSpan.End() case repository.FileActionRenamed: renameCtx, renameSpan := tracer.Start(ctx, "provisioning.sync.incremental.rename_resource_file") - name, gvk, err := repositoryResources.RenameResourceFile(renameCtx, change.PreviousPath, change.PreviousRef, change.Path, change.Ref) + name, oldFolderName, gvk, err := repositoryResources.RenameResourceFile(renameCtx, change.PreviousPath, change.PreviousRef, change.Path, change.Ref) if err != nil { renameSpan.RecordError(err) result.Error = fmt.Errorf("renaming resource file from %s to %s: %w", change.PreviousPath, change.Path, err) } result.Name = name - result.Resource = gvk.Kind + result.Kind = gvk.Kind result.Group = gvk.Group + + if oldFolderName != "" { + affectedFolders[safepath.Dir(change.Path)] = oldFolderName + } + renameSpan.End() case repository.FileActionIgnored: // do nothing @@ -128,5 +147,50 @@ func IncrementalSync(ctx context.Context, repo repository.Versioned, previousRef progress.SetMessage(ctx, "versioned changes replicated") + if len(affectedFolders) > 0 { + span.AddEvent("checking if impacted folders should be deleted", trace.WithAttributes(attribute.Int("affected_folders", len(affectedFolders)))) + if err := cleanupOrphanedFolders(ctx, repo, affectedFolders, repositoryResources, tracer); err != nil { + return tracing.Error(span, fmt.Errorf("cleanup orphaned folders: %w", err)) + } + } + + return nil +} + +// cleanupOrphanedFolders removes folders that no longer contain any resources in git after deletions have occurred. +func cleanupOrphanedFolders( + ctx context.Context, + repo repository.Versioned, + affectedFolders map[string]string, + repositoryResources resources.RepositoryResources, + tracer tracing.Tracer, +) error { + ctx, span := tracer.Start(ctx, "provisioning.sync.incremental.cleanup_orphaned_folders") + defer span.End() + + readerRepo, ok := repo.(repository.Reader) + if !ok { + span.RecordError(fmt.Errorf("repository does not implement Reader")) + return nil + } + + for path, folderName := range affectedFolders { + span.SetAttributes(attribute.String("folder", folderName)) + + // if we can no longer find the folder in git, then we can delete it from grafana + _, err := readerRepo.Read(ctx, path, "") + if err != nil && (errors.Is(err, repository.ErrFileNotFound) || apierrors.IsNotFound(err)) { + span.AddEvent("folder not found in git, removing from grafana") + if err := repositoryResources.RemoveFolder(ctx, folderName); err != nil { + span.RecordError(err) + } else { + span.AddEvent("successfully deleted") + } + continue + } + + span.AddEvent("folder still exists in git, continuing") + } + return nil } diff --git a/pkg/registry/apis/provisioning/jobs/sync/incremental_test.go b/pkg/registry/apis/provisioning/jobs/sync/incremental_test.go index 6562be15f3c..95d595e8c6c 100644 --- a/pkg/registry/apis/provisioning/jobs/sync/incremental_test.go +++ b/pkg/registry/apis/provisioning/jobs/sync/incremental_test.go @@ -5,13 +5,14 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime/schema" + "github.com/grafana/grafana/apps/provisioning/pkg/repository" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/registry/apis/provisioning/jobs" "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/runtime/schema" ) func TestIncrementalSync_ContextCancelled(t *testing.T) { @@ -131,11 +132,11 @@ func TestIncrementalSync(t *testing.T) { // Mock progress recording progress.On("Record", mock.Anything, jobs.JobResourceResult{ - Action: repository.FileActionCreated, - Path: "unsupported/path/", - Resource: resources.FolderResource.Resource, - Group: resources.FolderResource.Group, - Name: "test-folder", + Action: repository.FileActionCreated, + Path: "unsupported/path/", + Kind: resources.FolderKind.Kind, + Group: resources.FolderResource.Group, + Name: "test-folder", }).Return() progress.On("TooManyErrors").Return(nil) @@ -186,15 +187,15 @@ func TestIncrementalSync(t *testing.T) { // Mock resource deletion repoResources.On("RemoveResourceFromFile", mock.Anything, "dashboards/old.json", "old-ref"). - Return("old-dashboard", schema.GroupVersionKind{Kind: "Dashboard", Group: "dashboards"}, nil) + Return("old-dashboard", "", schema.GroupVersionKind{Kind: "Dashboard", Group: "dashboards"}, nil) // Mock progress recording progress.On("Record", mock.Anything, jobs.JobResourceResult{ - Action: repository.FileActionDeleted, - Path: "dashboards/old.json", - Name: "old-dashboard", - Resource: "Dashboard", - Group: "dashboards", + Action: repository.FileActionDeleted, + Path: "dashboards/old.json", + Name: "old-dashboard", + Kind: "Dashboard", + Group: "dashboards", }).Return() progress.On("TooManyErrors").Return(nil) @@ -222,15 +223,15 @@ func TestIncrementalSync(t *testing.T) { // Mock resource rename repoResources.On("RenameResourceFile", mock.Anything, "dashboards/old.json", "old-ref", "dashboards/new.json", "new-ref"). - Return("renamed-dashboard", schema.GroupVersionKind{Kind: "Dashboard", Group: "dashboards"}, nil) + Return("renamed-dashboard", "", schema.GroupVersionKind{Kind: "Dashboard", Group: "dashboards"}, nil) // Mock progress recording progress.On("Record", mock.Anything, jobs.JobResourceResult{ - Action: repository.FileActionRenamed, - Path: "dashboards/new.json", - Name: "renamed-dashboard", - Resource: "Dashboard", - Group: "dashboards", + Action: repository.FileActionRenamed, + Path: "dashboards/new.json", + Name: "renamed-dashboard", + Kind: "Dashboard", + Group: "dashboards", }).Return() progress.On("TooManyErrors").Return(nil) @@ -310,7 +311,7 @@ func TestIncrementalSync(t *testing.T) { return result.Action == repository.FileActionCreated && result.Path == "dashboards/test.json" && result.Name == "test-dashboard" && - result.Resource == "Dashboard" && + result.Kind == "Dashboard" && result.Group == "dashboards" && result.Error != nil && result.Error.Error() == "writing resource from file dashboards/test.json: write failed" @@ -339,14 +340,14 @@ func TestIncrementalSync(t *testing.T) { // Mock resource deletion error repoResources.On("RemoveResourceFromFile", mock.Anything, "dashboards/old.json", "old-ref"). - Return("old-dashboard", schema.GroupVersionKind{Kind: "Dashboard", Group: "dashboards"}, fmt.Errorf("delete failed")) + Return("old-dashboard", "", schema.GroupVersionKind{Kind: "Dashboard", Group: "dashboards"}, fmt.Errorf("delete failed")) // Mock progress recording with error progress.On("Record", mock.Anything, mock.MatchedBy(func(result jobs.JobResourceResult) bool { return result.Action == repository.FileActionDeleted && result.Path == "dashboards/old.json" && result.Name == "old-dashboard" && - result.Resource == "Dashboard" && + result.Kind == "Dashboard" && result.Group == "dashboards" && result.Error != nil && result.Error.Error() == "removing resource from file dashboards/old.json: delete failed" @@ -397,3 +398,126 @@ func TestIncrementalSync(t *testing.T) { }) } } + +type compositeRepo struct { + *repository.MockVersioned + *repository.MockReader +} + +func TestIncrementalSync_CleanupOrphanedFolders(t *testing.T) { + tests := []struct { + name string + setupMocks func(*compositeRepo, *resources.MockRepositoryResources, *jobs.MockJobProgressRecorder) + expectedError string + }{ + { + name: "delete folder when it no longer exists in git", + setupMocks: func(repo *compositeRepo, repoResources *resources.MockRepositoryResources, progress *jobs.MockJobProgressRecorder) { + changes := []repository.VersionedFileChange{ + { + Action: repository.FileActionDeleted, + Path: "dashboards/old.json", + PreviousRef: "old-ref", + }, + } + repo.MockVersioned.On("CompareFiles", mock.Anything, "old-ref", "new-ref").Return(changes, nil) + progress.On("SetTotal", mock.Anything, 1).Return() + progress.On("SetMessage", mock.Anything, "replicating versioned changes").Return() + progress.On("SetMessage", mock.Anything, "versioned changes replicated").Return() + repoResources.On("RemoveResourceFromFile", mock.Anything, "dashboards/old.json", "old-ref"). + Return("old-dashboard", "folder-uid", schema.GroupVersionKind{Kind: "Dashboard", Group: "dashboards"}, nil) + + // if the folder is not found in git, there should be a call to remove the folder from grafana + repo.MockReader.On("Read", mock.Anything, "dashboards/", ""). + Return((*repository.FileInfo)(nil), repository.ErrFileNotFound) + repoResources.On("RemoveFolder", mock.Anything, "folder-uid").Return(nil) + + progress.On("Record", mock.Anything, mock.Anything).Return() + progress.On("TooManyErrors").Return(nil) + }, + }, + { + name: "keep folder when it still exists in git", + setupMocks: func(repo *compositeRepo, repoResources *resources.MockRepositoryResources, progress *jobs.MockJobProgressRecorder) { + changes := []repository.VersionedFileChange{ + { + Action: repository.FileActionDeleted, + Path: "dashboards/old.json", + PreviousRef: "old-ref", + }, + } + repo.MockVersioned.On("CompareFiles", mock.Anything, "old-ref", "new-ref").Return(changes, nil) + progress.On("SetTotal", mock.Anything, 1).Return() + progress.On("SetMessage", mock.Anything, "replicating versioned changes").Return() + progress.On("SetMessage", mock.Anything, "versioned changes replicated").Return() + repoResources.On("RemoveResourceFromFile", mock.Anything, "dashboards/old.json", "old-ref"). + Return("old-dashboard", "folder-uid", schema.GroupVersionKind{Kind: "Dashboard", Group: "dashboards"}, nil) + // if the folder still exists in git, there should not be a call to delete it from grafana + repo.MockReader.On("Read", mock.Anything, "dashboards/", ""). + Return(&repository.FileInfo{}, nil) + + progress.On("Record", mock.Anything, mock.Anything).Return() + progress.On("TooManyErrors").Return(nil) + }, + }, + { + name: "delete multiple folders when they no longer exist in git", + setupMocks: func(repo *compositeRepo, repoResources *resources.MockRepositoryResources, progress *jobs.MockJobProgressRecorder) { + changes := []repository.VersionedFileChange{ + { + Action: repository.FileActionDeleted, + Path: "dashboards/old.json", + PreviousRef: "old-ref", + }, + { + Action: repository.FileActionDeleted, + Path: "alerts/old-alert.yaml", + PreviousRef: "old-ref", + }, + } + repo.MockVersioned.On("CompareFiles", mock.Anything, "old-ref", "new-ref").Return(changes, nil) + progress.On("SetTotal", mock.Anything, 2).Return() + progress.On("SetMessage", mock.Anything, "replicating versioned changes").Return() + progress.On("SetMessage", mock.Anything, "versioned changes replicated").Return() + repoResources.On("RemoveResourceFromFile", mock.Anything, "dashboards/old.json", "old-ref"). + Return("old-dashboard", "folder-uid-1", schema.GroupVersionKind{Kind: "Dashboard", Group: "dashboards"}, nil) + repoResources.On("RemoveResourceFromFile", mock.Anything, "alerts/old-alert.yaml", "old-ref"). + Return("old-alert", "folder-uid-2", schema.GroupVersionKind{Kind: "Alert", Group: "alerts"}, nil) + + // both not found in git, both should be deleted + repo.MockReader.On("Read", mock.Anything, "dashboards/", ""). + Return((*repository.FileInfo)(nil), repository.ErrFileNotFound) + repo.MockReader.On("Read", mock.Anything, "alerts/", ""). + Return((*repository.FileInfo)(nil), repository.ErrFileNotFound) + repoResources.On("RemoveFolder", mock.Anything, "folder-uid-1").Return(nil) + repoResources.On("RemoveFolder", mock.Anything, "folder-uid-2").Return(nil) + + progress.On("Record", mock.Anything, mock.Anything).Return() + progress.On("TooManyErrors").Return(nil) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockVersioned := repository.NewMockVersioned(t) + mockReader := repository.NewMockReader(t) + repo := &compositeRepo{ + MockVersioned: mockVersioned, + MockReader: mockReader, + } + repoResources := resources.NewMockRepositoryResources(t) + progress := jobs.NewMockJobProgressRecorder(t) + + tt.setupMocks(repo, repoResources, progress) + + err := IncrementalSync(context.Background(), repo, "old-ref", "new-ref", repoResources, progress, tracing.NewNoopTracerService()) + + if tt.expectedError != "" { + require.EqualError(t, err, tt.expectedError) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/pkg/registry/apis/provisioning/jobs/sync/worker.go b/pkg/registry/apis/provisioning/jobs/sync/worker.go index 09ed5859cc7..c5df4553e04 100644 --- a/pkg/registry/apis/provisioning/jobs/sync/worker.go +++ b/pkg/registry/apis/provisioning/jobs/sync/worker.go @@ -139,15 +139,17 @@ func (r *SyncWorker) Process(ctx context.Context, repo repository.Repository, jo if err != nil { setupSpan.End() logger.Error("failed to create repository resources client", "error", err) - err = fmt.Errorf("create repository resources client: %w", err) - return tracing.Error(span, err) + setupError := fmt.Errorf("create repository resources client: %w", err) + progress.Complete(ctx, setupError) + return tracing.Error(span, setupError) } clients, err := r.clients.Clients(setupCtx, cfg.Namespace) if err != nil { setupSpan.End() logger.Error("failed to get clients for the repository", "error", err) - err = fmt.Errorf("get clients for %s: %w", cfg.Name, err) - return tracing.Error(span, err) + setupError := fmt.Errorf("get clients for %s: %w", cfg.Name, err) + progress.Complete(ctx, setupError) + return tracing.Error(span, setupError) } setupSpan.End() diff --git a/pkg/registry/apis/provisioning/jobs/sync/worker_test.go b/pkg/registry/apis/provisioning/jobs/sync/worker_test.go index 090d4deac87..bd8b094b4f6 100644 --- a/pkg/registry/apis/provisioning/jobs/sync/worker_test.go +++ b/pkg/registry/apis/provisioning/jobs/sync/worker_test.go @@ -153,10 +153,15 @@ func TestSyncWorker_Process(t *testing.T) { // Initial status update succeeds pr.On("SetMessage", mock.Anything, "update sync status at start").Return() - rpf.On("Execute", mock.Anything, repoConfig, mock.Anything).Return(nil) + rpf.On("Execute", mock.Anything, repoConfig, mock.Anything).Return(nil).Once() // Repository resources creation fails rrf.On("Client", mock.Anything, mock.Anything).Return(nil, errors.New("failed to create repository resources client")) + + // Progress.Complete should be called with the error + pr.On("Complete", mock.Anything, mock.MatchedBy(func(err error) bool { + return err != nil && err.Error() == "create repository resources client: failed to create repository resources client" + })).Return(provisioning.JobStatus{State: provisioning.JobStateError}) }, expectedError: "create repository resources client: failed to create repository resources client", }, @@ -185,13 +190,18 @@ func TestSyncWorker_Process(t *testing.T) { // Initial status update succeeds pr.On("SetMessage", mock.Anything, "update sync status at start").Return() - rpf.On("Execute", mock.Anything, repoConfig, mock.Anything).Return(nil) + rpf.On("Execute", mock.Anything, repoConfig, mock.Anything).Return(nil).Once() // Repository resources creation succeeds rrf.On("Client", mock.Anything, mock.Anything).Return(&resources.MockRepositoryResources{}, nil) // Getting clients for namespace fails cf.On("Clients", mock.Anything, "test-namespace").Return(nil, errors.New("failed to get clients")) + + // Progress.Complete should be called with the error + pr.On("Complete", mock.Anything, mock.MatchedBy(func(err error) bool { + return err != nil && err.Error() == "get clients for test-repo: failed to get clients" + })).Return(provisioning.JobStatus{State: provisioning.JobStateError}) }, expectedError: "get clients for test-repo: failed to get clients", }, diff --git a/pkg/registry/apis/provisioning/resources/client.go b/pkg/registry/apis/provisioning/resources/client.go index 6a68a8add31..f12bcf7b053 100644 --- a/pkg/registry/apis/provisioning/resources/client.go +++ b/pkg/registry/apis/provisioning/resources/client.go @@ -22,6 +22,7 @@ import ( var ( UserResource = iam.UserResourceInfo.GroupVersionResource() FolderResource = folders.FolderResourceInfo.GroupVersionResource() + FolderKind = folders.FolderResourceInfo.GroupVersionKind() DashboardResource = dashboardV1.DashboardResourceInfo.GroupVersionResource() DashboardResourceV2alpha1 = dashboardV2alpha1.DashboardResourceInfo.GroupVersionResource() DashboardResourceV2beta1 = dashboardV2beta1.DashboardResourceInfo.GroupVersionResource() diff --git a/pkg/registry/apis/provisioning/resources/folders.go b/pkg/registry/apis/provisioning/resources/folders.go index ba6388b31cf..b74ab67976a 100644 --- a/pkg/registry/apis/provisioning/resources/folders.go +++ b/pkg/registry/apis/provisioning/resources/folders.go @@ -175,6 +175,10 @@ func (fm *FolderManager) GetFolder(ctx context.Context, name string) (*unstructu return fm.client.Get(ctx, name, metav1.GetOptions{}) } +func (fm *FolderManager) RemoveFolder(ctx context.Context, name string) error { + return fm.client.Delete(ctx, name, metav1.DeleteOptions{}) +} + // ReplicateTree replicates the folder tree to the repository. // The function fn is called for each folder. // If the folder already exists, the function is called with created set to false. diff --git a/pkg/registry/apis/provisioning/resources/repository.go b/pkg/registry/apis/provisioning/resources/repository.go index 67236d93cd5..6a9e16fc5a2 100644 --- a/pkg/registry/apis/provisioning/resources/repository.go +++ b/pkg/registry/apis/provisioning/resources/repository.go @@ -28,13 +28,14 @@ type RepositoryResources interface { EnsureFolderPathExist(ctx context.Context, filePath string) (parent string, err error) EnsureFolderExists(ctx context.Context, folder Folder, parentID string) error EnsureFolderTreeExists(ctx context.Context, ref, path string, tree FolderTree, fn func(folder Folder, created bool, err error) error) error + RemoveFolder(ctx context.Context, folderName string) error // File from Resource WriteResourceFileFromObject(ctx context.Context, obj *unstructured.Unstructured, options WriteOptions) (string, error) // Resource from file WriteResourceFromFile(ctx context.Context, path, ref string) (string, schema.GroupVersionKind, error) - RemoveResourceFromFile(ctx context.Context, path, ref string) (string, schema.GroupVersionKind, error) + RemoveResourceFromFile(ctx context.Context, path, ref string) (string, string, schema.GroupVersionKind, error) FindResourcePath(ctx context.Context, name string, gvk schema.GroupVersionKind) (string, error) - RenameResourceFile(ctx context.Context, path, previousRef, newPath, newRef string) (string, schema.GroupVersionKind, error) + RenameResourceFile(ctx context.Context, path, previousRef, newPath, newRef string) (string, string, schema.GroupVersionKind, error) // Stats Stats(ctx context.Context) (*provisioning.ResourceStats, error) List(ctx context.Context) (*provisioning.ResourceList, error) diff --git a/pkg/registry/apis/provisioning/resources/repository_resources_mock.go b/pkg/registry/apis/provisioning/resources/repository_resources_mock.go index 978d91f1d4d..1628971ff9e 100644 --- a/pkg/registry/apis/provisioning/resources/repository_resources_mock.go +++ b/pkg/registry/apis/provisioning/resources/repository_resources_mock.go @@ -297,8 +297,55 @@ func (_c *MockRepositoryResources_List_Call) RunAndReturn(run func(context.Conte return _c } +// RemoveFolder provides a mock function with given fields: ctx, folderName +func (_m *MockRepositoryResources) RemoveFolder(ctx context.Context, folderName string) error { + ret := _m.Called(ctx, folderName) + + if len(ret) == 0 { + panic("no return value specified for RemoveFolder") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = rf(ctx, folderName) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MockRepositoryResources_RemoveFolder_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RemoveFolder' +type MockRepositoryResources_RemoveFolder_Call struct { + *mock.Call +} + +// RemoveFolder is a helper method to define mock.On call +// - ctx context.Context +// - folderName string +func (_e *MockRepositoryResources_Expecter) RemoveFolder(ctx interface{}, folderName interface{}) *MockRepositoryResources_RemoveFolder_Call { + return &MockRepositoryResources_RemoveFolder_Call{Call: _e.mock.On("RemoveFolder", ctx, folderName)} +} + +func (_c *MockRepositoryResources_RemoveFolder_Call) Run(run func(ctx context.Context, folderName string)) *MockRepositoryResources_RemoveFolder_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string)) + }) + return _c +} + +func (_c *MockRepositoryResources_RemoveFolder_Call) Return(_a0 error) *MockRepositoryResources_RemoveFolder_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockRepositoryResources_RemoveFolder_Call) RunAndReturn(run func(context.Context, string) error) *MockRepositoryResources_RemoveFolder_Call { + _c.Call.Return(run) + return _c +} + // RemoveResourceFromFile provides a mock function with given fields: ctx, path, ref -func (_m *MockRepositoryResources) RemoveResourceFromFile(ctx context.Context, path string, ref string) (string, schema.GroupVersionKind, error) { +func (_m *MockRepositoryResources) RemoveResourceFromFile(ctx context.Context, path string, ref string) (string, string, schema.GroupVersionKind, error) { ret := _m.Called(ctx, path, ref) if len(ret) == 0 { @@ -306,9 +353,10 @@ func (_m *MockRepositoryResources) RemoveResourceFromFile(ctx context.Context, p } var r0 string - var r1 schema.GroupVersionKind - var r2 error - if rf, ok := ret.Get(0).(func(context.Context, string, string) (string, schema.GroupVersionKind, error)); ok { + var r1 string + var r2 schema.GroupVersionKind + var r3 error + if rf, ok := ret.Get(0).(func(context.Context, string, string) (string, string, schema.GroupVersionKind, error)); ok { return rf(ctx, path, ref) } if rf, ok := ret.Get(0).(func(context.Context, string, string) string); ok { @@ -317,19 +365,25 @@ func (_m *MockRepositoryResources) RemoveResourceFromFile(ctx context.Context, p r0 = ret.Get(0).(string) } - if rf, ok := ret.Get(1).(func(context.Context, string, string) schema.GroupVersionKind); ok { + if rf, ok := ret.Get(1).(func(context.Context, string, string) string); ok { r1 = rf(ctx, path, ref) } else { - r1 = ret.Get(1).(schema.GroupVersionKind) + r1 = ret.Get(1).(string) } - if rf, ok := ret.Get(2).(func(context.Context, string, string) error); ok { + if rf, ok := ret.Get(2).(func(context.Context, string, string) schema.GroupVersionKind); ok { r2 = rf(ctx, path, ref) } else { - r2 = ret.Error(2) + r2 = ret.Get(2).(schema.GroupVersionKind) } - return r0, r1, r2 + if rf, ok := ret.Get(3).(func(context.Context, string, string) error); ok { + r3 = rf(ctx, path, ref) + } else { + r3 = ret.Error(3) + } + + return r0, r1, r2, r3 } // MockRepositoryResources_RemoveResourceFromFile_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RemoveResourceFromFile' @@ -352,18 +406,18 @@ func (_c *MockRepositoryResources_RemoveResourceFromFile_Call) Run(run func(ctx return _c } -func (_c *MockRepositoryResources_RemoveResourceFromFile_Call) Return(_a0 string, _a1 schema.GroupVersionKind, _a2 error) *MockRepositoryResources_RemoveResourceFromFile_Call { - _c.Call.Return(_a0, _a1, _a2) +func (_c *MockRepositoryResources_RemoveResourceFromFile_Call) Return(_a0 string, _a1 string, _a2 schema.GroupVersionKind, _a3 error) *MockRepositoryResources_RemoveResourceFromFile_Call { + _c.Call.Return(_a0, _a1, _a2, _a3) return _c } -func (_c *MockRepositoryResources_RemoveResourceFromFile_Call) RunAndReturn(run func(context.Context, string, string) (string, schema.GroupVersionKind, error)) *MockRepositoryResources_RemoveResourceFromFile_Call { +func (_c *MockRepositoryResources_RemoveResourceFromFile_Call) RunAndReturn(run func(context.Context, string, string) (string, string, schema.GroupVersionKind, error)) *MockRepositoryResources_RemoveResourceFromFile_Call { _c.Call.Return(run) return _c } // RenameResourceFile provides a mock function with given fields: ctx, path, previousRef, newPath, newRef -func (_m *MockRepositoryResources) RenameResourceFile(ctx context.Context, path string, previousRef string, newPath string, newRef string) (string, schema.GroupVersionKind, error) { +func (_m *MockRepositoryResources) RenameResourceFile(ctx context.Context, path string, previousRef string, newPath string, newRef string) (string, string, schema.GroupVersionKind, error) { ret := _m.Called(ctx, path, previousRef, newPath, newRef) if len(ret) == 0 { @@ -371,9 +425,10 @@ func (_m *MockRepositoryResources) RenameResourceFile(ctx context.Context, path } var r0 string - var r1 schema.GroupVersionKind - var r2 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string) (string, schema.GroupVersionKind, error)); ok { + var r1 string + var r2 schema.GroupVersionKind + var r3 error + if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string) (string, string, schema.GroupVersionKind, error)); ok { return rf(ctx, path, previousRef, newPath, newRef) } if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string) string); ok { @@ -382,19 +437,25 @@ func (_m *MockRepositoryResources) RenameResourceFile(ctx context.Context, path r0 = ret.Get(0).(string) } - if rf, ok := ret.Get(1).(func(context.Context, string, string, string, string) schema.GroupVersionKind); ok { + if rf, ok := ret.Get(1).(func(context.Context, string, string, string, string) string); ok { r1 = rf(ctx, path, previousRef, newPath, newRef) } else { - r1 = ret.Get(1).(schema.GroupVersionKind) + r1 = ret.Get(1).(string) } - if rf, ok := ret.Get(2).(func(context.Context, string, string, string, string) error); ok { + if rf, ok := ret.Get(2).(func(context.Context, string, string, string, string) schema.GroupVersionKind); ok { r2 = rf(ctx, path, previousRef, newPath, newRef) } else { - r2 = ret.Error(2) + r2 = ret.Get(2).(schema.GroupVersionKind) } - return r0, r1, r2 + if rf, ok := ret.Get(3).(func(context.Context, string, string, string, string) error); ok { + r3 = rf(ctx, path, previousRef, newPath, newRef) + } else { + r3 = ret.Error(3) + } + + return r0, r1, r2, r3 } // MockRepositoryResources_RenameResourceFile_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RenameResourceFile' @@ -419,12 +480,12 @@ func (_c *MockRepositoryResources_RenameResourceFile_Call) Run(run func(ctx cont return _c } -func (_c *MockRepositoryResources_RenameResourceFile_Call) Return(_a0 string, _a1 schema.GroupVersionKind, _a2 error) *MockRepositoryResources_RenameResourceFile_Call { - _c.Call.Return(_a0, _a1, _a2) +func (_c *MockRepositoryResources_RenameResourceFile_Call) Return(_a0 string, _a1 string, _a2 schema.GroupVersionKind, _a3 error) *MockRepositoryResources_RenameResourceFile_Call { + _c.Call.Return(_a0, _a1, _a2, _a3) return _c } -func (_c *MockRepositoryResources_RenameResourceFile_Call) RunAndReturn(run func(context.Context, string, string, string, string) (string, schema.GroupVersionKind, error)) *MockRepositoryResources_RenameResourceFile_Call { +func (_c *MockRepositoryResources_RenameResourceFile_Call) RunAndReturn(run func(context.Context, string, string, string, string) (string, string, schema.GroupVersionKind, error)) *MockRepositoryResources_RenameResourceFile_Call { _c.Call.Return(run) return _c } diff --git a/pkg/registry/apis/provisioning/resources/resources.go b/pkg/registry/apis/provisioning/resources/resources.go index 027f04671ff..b66e16917da 100644 --- a/pkg/registry/apis/provisioning/resources/resources.go +++ b/pkg/registry/apis/provisioning/resources/resources.go @@ -238,44 +238,63 @@ func (r *ResourcesManager) WriteResourceFromFile(ctx context.Context, path strin return parsed.Obj.GetName(), parsed.GVK, err } -func (r *ResourcesManager) RenameResourceFile(ctx context.Context, previousPath, previousRef, newPath, newRef string) (string, schema.GroupVersionKind, error) { - name, gvk, err := r.RemoveResourceFromFile(ctx, previousPath, previousRef) +func (r *ResourcesManager) RenameResourceFile(ctx context.Context, previousPath, previousRef, newPath, newRef string) (string, string, schema.GroupVersionKind, error) { + name, oldFolderName, gvk, err := r.RemoveResourceFromFile(ctx, previousPath, previousRef) if err != nil { - return name, gvk, fmt.Errorf("failed to remove resource: %w", err) + return name, oldFolderName, gvk, fmt.Errorf("failed to remove resource: %w", err) } - return r.WriteResourceFromFile(ctx, newPath, newRef) + newName, gvk, err := r.WriteResourceFromFile(ctx, newPath, newRef) + if err != nil { + return name, oldFolderName, gvk, fmt.Errorf("failed to write resource: %w", err) + } + + return newName, oldFolderName, gvk, nil } -func (r *ResourcesManager) RemoveResourceFromFile(ctx context.Context, path string, ref string) (string, schema.GroupVersionKind, error) { +func (r *ResourcesManager) RemoveResourceFromFile(ctx context.Context, path string, ref string) (string, string, schema.GroupVersionKind, error) { info, err := r.repo.Read(ctx, path, ref) if err != nil { - return "", schema.GroupVersionKind{}, fmt.Errorf("failed to read file: %w", err) + return "", "", schema.GroupVersionKind{}, fmt.Errorf("failed to read file: %w", err) } obj, gvk, _ := DecodeYAMLObject(bytes.NewBuffer(info.Data)) if obj == nil { - return "", schema.GroupVersionKind{}, fmt.Errorf("no object found") + return "", "", schema.GroupVersionKind{}, fmt.Errorf("no object found") } objName := obj.GetName() if objName == "" { - return "", schema.GroupVersionKind{}, ErrMissingName + return "", "", schema.GroupVersionKind{}, ErrMissingName } client, _, err := r.clients.ForKind(ctx, *gvk) if err != nil { - return "", schema.GroupVersionKind{}, fmt.Errorf("unable to get client for deleted object: %w", err) + return "", "", schema.GroupVersionKind{}, fmt.Errorf("unable to get client for deleted object: %w", err) } + // the folder annotation is not stored in the git file, so we need to get it from grafana + grafanaObj, err := client.Get(ctx, objName, metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + return objName, "", schema.GroupVersionKind{}, nil // Already deleted or simply non-existing, nothing to do + } + return "", "", schema.GroupVersionKind{}, fmt.Errorf("unable to get grafana object: %w", err) + } + meta, err := utils.MetaAccessor(grafanaObj) + if err != nil { + return "", "", schema.GroupVersionKind{}, fmt.Errorf("unable to get meta accessor: %w", err) + } + folderName := meta.GetFolder() + err = client.Delete(ctx, objName, metav1.DeleteOptions{}) if err != nil { if apierrors.IsNotFound(err) { - return objName, schema.GroupVersionKind{}, nil // Already deleted or simply non-existing, nothing to do + return objName, folderName, schema.GroupVersionKind{}, nil // Already deleted or simply non-existing, nothing to do } - return "", schema.GroupVersionKind{}, fmt.Errorf("failed to delete: %w", err) + return "", "", schema.GroupVersionKind{}, fmt.Errorf("failed to delete: %w", err) } - return objName, schema.GroupVersionKind{}, nil + return objName, folderName, schema.GroupVersionKind{}, nil } diff --git a/pkg/registry/apis/secret/contracts/encryption.go b/pkg/registry/apis/secret/contracts/encryption.go index 73a4952cac2..f24fe73a544 100644 --- a/pkg/registry/apis/secret/contracts/encryption.go +++ b/pkg/registry/apis/secret/contracts/encryption.go @@ -1,10 +1,6 @@ package contracts -import ( - "context" - - "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" -) +import "context" // EncryptionManager is an envelope encryption service in charge of encrypting/decrypting secrets. type EncryptionManager interface { @@ -12,23 +8,17 @@ type EncryptionManager interface { // For those specific use cases where the encryption operation cannot be moved outside // the database transaction, look at database-specific methods present at the specific // implementation present at manager.EncryptionService. - Encrypt(ctx context.Context, namespace xkube.Namespace, payload []byte) (EncryptedPayload, error) - Decrypt(ctx context.Context, namespace xkube.Namespace, payload EncryptedPayload) ([]byte, error) -} - -type EncryptedPayload struct { - DataKeyID string - EncryptedData []byte + Encrypt(ctx context.Context, namespace string, payload []byte) ([]byte, error) + Decrypt(ctx context.Context, namespace string, payload []byte) ([]byte, error) } type EncryptedValue struct { - EncryptedPayload - - Namespace string - Name string - Version int64 - Created int64 - Updated int64 + Namespace string + Name string + Version int64 + EncryptedData []byte + Created int64 + Updated int64 } // ListOpts defines pagination options for listing encrypted values. @@ -38,10 +28,10 @@ type ListOpts struct { } type EncryptedValueStorage interface { - Create(ctx context.Context, namespace xkube.Namespace, name string, version int64, encryptedData EncryptedPayload) (*EncryptedValue, error) - Update(ctx context.Context, namespace xkube.Namespace, name string, version int64, encryptedData EncryptedPayload) error - Get(ctx context.Context, namespace xkube.Namespace, name string, version int64) (*EncryptedValue, error) - Delete(ctx context.Context, namespace xkube.Namespace, name string, version int64) error + Create(ctx context.Context, namespace, name string, version int64, encryptedData []byte) (*EncryptedValue, error) + Update(ctx context.Context, namespace, name string, version int64, encryptedData []byte) error + Get(ctx context.Context, namespace, name string, version int64) (*EncryptedValue, error) + Delete(ctx context.Context, namespace, name string, version int64) error } type GlobalEncryptedValueStorage interface { @@ -49,10 +39,6 @@ type GlobalEncryptedValueStorage interface { CountAll(ctx context.Context, untilTime *int64) (int64, error) } -type EncryptedValueMigrationExecutor interface { - Execute(ctx context.Context) (int, error) -} - type ConsolidationService interface { Consolidate(ctx context.Context) error } diff --git a/pkg/registry/apis/secret/contracts/keeper.go b/pkg/registry/apis/secret/contracts/keeper.go index a204bb5b1a7..5558a4db3b5 100644 --- a/pkg/registry/apis/secret/contracts/keeper.go +++ b/pkg/registry/apis/secret/contracts/keeper.go @@ -96,10 +96,10 @@ func (s ExternalID) String() string { // Keeper is the interface for secret keepers. type Keeper interface { - Store(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace xkube.Namespace, name string, version int64, exposedValueOrRef string) (ExternalID, error) - Update(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace xkube.Namespace, name string, version int64, exposedValueOrRef string) error - Expose(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace xkube.Namespace, name string, version int64) (secretv1beta1.ExposedSecureValue, error) - Delete(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace xkube.Namespace, name string, version int64) error + Store(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace, name string, version int64, exposedValueOrRef string) (ExternalID, error) + Update(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace, name string, version int64, exposedValueOrRef string) error + Expose(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace, name string, version int64) (secretv1beta1.ExposedSecureValue, error) + Delete(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace, name string, version int64) error } // Service is the interface for secret keeper services. diff --git a/pkg/registry/apis/secret/encryption/manager/manager.go b/pkg/registry/apis/secret/encryption/manager/manager.go index 024c8aca05a..f48b350c1bd 100644 --- a/pkg/registry/apis/secret/encryption/manager/manager.go +++ b/pkg/registry/apis/secret/encryption/manager/manager.go @@ -1,8 +1,10 @@ package manager import ( + "bytes" "context" "crypto/rand" + "encoding/base64" "errors" "fmt" "strconv" @@ -18,10 +20,13 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/registry/apis/secret/encryption" "github.com/grafana/grafana/pkg/registry/apis/secret/encryption/cipher" - "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" "github.com/grafana/grafana/pkg/util" ) +const ( + keyIdDelimiter = '#' +) + type EncryptionManager struct { tracer trace.Tracer store contracts.DataKeyStorage @@ -94,9 +99,12 @@ func (s *EncryptionManager) registerUsageMetrics() { }) } -func (s *EncryptionManager) Encrypt(ctx context.Context, namespace xkube.Namespace, payload []byte) (contracts.EncryptedPayload, error) { +// TODO: Why do we need to use a global variable for this? +var b64 = base64.RawStdEncoding + +func (s *EncryptionManager) Encrypt(ctx context.Context, namespace string, payload []byte) ([]byte, error) { ctx, span := s.tracer.Start(ctx, "EnvelopeEncryptionManager.Encrypt", trace.WithAttributes( - attribute.String("namespace", namespace.String()), + attribute.String("namespace", namespace), )) defer span.End() @@ -120,30 +128,34 @@ func (s *EncryptionManager) Encrypt(ctx context.Context, namespace xkube.Namespa id, dataKey, err = s.currentDataKey(ctx, namespace, label) if err != nil { s.log.Error("Failed to get current data key", "error", err, "label", label) - return contracts.EncryptedPayload{}, err + return nil, err } var encrypted []byte encrypted, err = s.cipher.Encrypt(ctx, payload, string(dataKey)) if err != nil { s.log.Error("Failed to encrypt secret", "error", err) - return contracts.EncryptedPayload{}, err + return nil, err } - encryptedPayload := contracts.EncryptedPayload{ - DataKeyID: id, - EncryptedData: encrypted, - } + prefix := make([]byte, b64.EncodedLen(len(id))+2) + b64.Encode(prefix[1:], []byte(id)) + prefix[0] = keyIdDelimiter + prefix[len(prefix)-1] = keyIdDelimiter - return encryptedPayload, nil + blob := make([]byte, len(prefix)+len(encrypted)) + copy(blob, prefix) + copy(blob[len(prefix):], encrypted) + + return blob, nil } // currentDataKey looks up for current data key in cache or database by name, and decrypts it. // If there's no current data key in cache nor in database it generates a new random data key, // and stores it into both the in-memory cache and database (encrypted by the encryption provider). -func (s *EncryptionManager) currentDataKey(ctx context.Context, namespace xkube.Namespace, label string) (string, []byte, error) { +func (s *EncryptionManager) currentDataKey(ctx context.Context, namespace string, label string) (string, []byte, error) { ctx, span := s.tracer.Start(ctx, "EnvelopeEncryptionManager.CurrentDataKey", trace.WithAttributes( - attribute.String("namespace", namespace.String()), + attribute.String("namespace", namespace), attribute.String("label", label), )) defer span.End() @@ -154,14 +166,14 @@ func (s *EncryptionManager) currentDataKey(ctx context.Context, namespace xkube. defer s.mtx.Unlock() // We try to fetch the data key, either from cache or database - id, dataKey, err := s.dataKeyByLabel(ctx, namespace.String(), label) + id, dataKey, err := s.dataKeyByLabel(ctx, namespace, label) if err != nil { return "", nil, err } // If no existing data key was found, create a new one if dataKey == nil { - id, dataKey, err = s.newDataKey(ctx, namespace.String(), label) + id, dataKey, err = s.newDataKey(ctx, namespace, label) if err != nil { return "", nil, err } @@ -252,9 +264,9 @@ func newRandomDataKey() ([]byte, error) { return rawDataKey, nil } -func (s *EncryptionManager) Decrypt(ctx context.Context, namespace xkube.Namespace, payload contracts.EncryptedPayload) ([]byte, error) { +func (s *EncryptionManager) Decrypt(ctx context.Context, namespace string, payload []byte) ([]byte, error) { ctx, span := s.tracer.Start(ctx, "EnvelopeEncryptionManager.Decrypt", trace.WithAttributes( - attribute.String("namespace", namespace.String()), + attribute.String("namespace", namespace), )) defer span.End() @@ -273,28 +285,50 @@ func (s *EncryptionManager) Decrypt(ctx context.Context, namespace xkube.Namespa } }() - if len(payload.EncryptedData) == 0 { + if len(payload) == 0 { err = fmt.Errorf("unable to decrypt empty payload") return nil, err } - if payload.DataKeyID == "" { - err = fmt.Errorf("unable to decrypt empty data key id") + payload = payload[1:] + endOfKey := bytes.Index(payload, []byte{keyIdDelimiter}) + if endOfKey == -1 { + err = fmt.Errorf("could not find valid key id in encrypted payload") + return nil, err + } + b64Key := payload[:endOfKey] + payload = payload[endOfKey+1:] + keyId := make([]byte, b64.DecodedLen(len(b64Key))) + _, err = b64.Decode(keyId, b64Key) + if err != nil { return nil, err } - dataKey, err := s.dataKeyById(ctx, namespace.String(), payload.DataKeyID) + dataKey, err := s.dataKeyById(ctx, namespace, string(keyId)) if err != nil { - s.log.FromContext(ctx).Error("Failed to lookup data key by id", "id", payload.DataKeyID, "error", err) + s.log.FromContext(ctx).Error("Failed to lookup data key by id", "id", string(keyId), "error", err) return nil, err } var decrypted []byte - decrypted, err = s.cipher.Decrypt(ctx, payload.EncryptedData, string(dataKey)) + decrypted, err = s.cipher.Decrypt(ctx, payload, string(dataKey)) return decrypted, err } +func (s *EncryptionManager) GetDecryptedValue(ctx context.Context, namespace string, sjd map[string][]byte, key, fallback string) string { + if value, ok := sjd[key]; ok { + decryptedData, err := s.Decrypt(ctx, namespace, value) + if err != nil { + return fallback + } + + return string(decryptedData) + } + + return fallback +} + // dataKeyById looks up for data key in the database and returns it decrypted. func (s *EncryptionManager) dataKeyById(ctx context.Context, namespace, id string) ([]byte, error) { ctx, span := s.tracer.Start(ctx, "EnvelopeEncryptionManager.GetDataKey", trace.WithAttributes( diff --git a/pkg/registry/apis/secret/encryption/manager/manager_test.go b/pkg/registry/apis/secret/encryption/manager/manager_test.go index dc63fb461c9..a6c1c06bcfa 100644 --- a/pkg/registry/apis/secret/encryption/manager/manager_test.go +++ b/pkg/registry/apis/secret/encryption/manager/manager_test.go @@ -17,7 +17,6 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/secret/encryption" "github.com/grafana/grafana/pkg/registry/apis/secret/encryption/cipher/service" osskmsproviders "github.com/grafana/grafana/pkg/registry/apis/secret/encryption/kmsproviders" - "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/secret/database" @@ -35,7 +34,7 @@ func TestMain(m *testing.M) { func TestEncryptionService_EnvelopeEncryption(t *testing.T) { svc := setupTestService(t) ctx := context.Background() - namespace := xkube.Namespace("test-namespace") + namespace := "test-namespace" t.Run("encrypting should create DEK", func(t *testing.T) { plaintext := []byte("very secret string") @@ -47,7 +46,7 @@ func TestEncryptionService_EnvelopeEncryption(t *testing.T) { require.NoError(t, err) assert.Equal(t, plaintext, decrypted) - keys, err := svc.store.ListDataKeys(ctx, namespace.String()) + keys, err := svc.store.ListDataKeys(ctx, namespace) require.NoError(t, err) assert.Equal(t, len(keys), 1) }) @@ -62,7 +61,7 @@ func TestEncryptionService_EnvelopeEncryption(t *testing.T) { require.NoError(t, err) assert.Equal(t, plaintext, decrypted) - keys, err := svc.store.ListDataKeys(ctx, namespace.String()) + keys, err := svc.store.ListDataKeys(ctx, namespace) require.NoError(t, err) assert.Equal(t, len(keys), 1) }) @@ -213,7 +212,7 @@ func TestEncryptionService_UseCurrentProvider(t *testing.T) { } encryptionManager.providerConfig.CurrentProvider = encryption.ProviderID("fakeProvider.v1") - namespace := xkube.Namespace("test-namespace") + namespace := "test-namespace" encrypted, _ := encryptionManager.Encrypt(context.Background(), namespace, []byte{}) assert.True(t, fake.encryptCalled) assert.False(t, fake.decryptCalled) @@ -242,7 +241,7 @@ func TestEncryptionService_UseCurrentProvider(t *testing.T) { func TestEncryptionService_SecretKeyVersionUpgrade(t *testing.T) { ctx := context.Background() - namespace := xkube.Namespace("test-namespace") + namespace := "test-namespace" // Generate random keys for testing oldKey := util.GenerateShortUID() + util.GenerateShortUID() // 32 chars @@ -417,30 +416,16 @@ func (p *fakeProvider) Decrypt(_ context.Context, _ []byte) ([]byte, error) { func TestEncryptionService_Decrypt(t *testing.T) { ctx := context.Background() - namespace := xkube.Namespace("test-namespace") + namespace := "test-namespace" t.Run("empty payload should fail", func(t *testing.T) { svc := setupTestService(t) - _, err := svc.Decrypt(context.Background(), namespace, contracts.EncryptedPayload{ - DataKeyID: "test-data-key-id", - EncryptedData: []byte(""), - }) + _, err := svc.Decrypt(context.Background(), namespace, []byte("")) require.Error(t, err) assert.Equal(t, "unable to decrypt empty payload", err.Error()) }) - t.Run("empty data key id should fail", func(t *testing.T) { - svc := setupTestService(t) - _, err := svc.Decrypt(context.Background(), namespace, contracts.EncryptedPayload{ - DataKeyID: "", - EncryptedData: []byte("some payload"), - }) - require.Error(t, err) - - assert.Equal(t, "unable to decrypt empty data key id", err.Error()) - }) - t.Run("ee encrypted payload with ee enabled should work", func(t *testing.T) { svc := setupTestService(t) ciphertext, err := svc.Encrypt(ctx, namespace, []byte("grafana")) @@ -457,7 +442,7 @@ func TestIntegration_SecretsService(t *testing.T) { ctx := context.Background() someData := []byte(`some-data`) - namespace := xkube.Namespace("test-namespace") + namespace := "test-namespace" tcs := map[string]func(*testing.T, db.DB, contracts.EncryptionManager){ "regular": func(t *testing.T, _ db.DB, svc contracts.EncryptionManager) { @@ -577,7 +562,7 @@ func TestIntegration_SecretsService(t *testing.T) { require.NoError(t, err) ctx := context.Background() - namespace := xkube.Namespace("test-namespace") + namespace := "test-namespace" // Here's what actually matters and varies on each test: look at the test case name. // diff --git a/pkg/registry/apis/secret/garbagecollectionworker/worker.go b/pkg/registry/apis/secret/garbagecollectionworker/worker.go index e10bbed600e..4754f06c02d 100644 --- a/pkg/registry/apis/secret/garbagecollectionworker/worker.go +++ b/pkg/registry/apis/secret/garbagecollectionworker/worker.go @@ -104,7 +104,7 @@ func (w *Worker) Cleanup(ctx context.Context, sv *secretv1beta1.SecureValue) err } // Keeper deletion is idempotent - if err := keeper.Delete(ctx, keeperCfg, xkube.Namespace(sv.Namespace), sv.Name, sv.Status.Version); err != nil { + if err := keeper.Delete(ctx, keeperCfg, sv.Namespace, sv.Name, sv.Status.Version); err != nil { return fmt.Errorf("deleting secure value from keeper: %w", err) } diff --git a/pkg/registry/apis/secret/garbagecollectionworker/worker_test.go b/pkg/registry/apis/secret/garbagecollectionworker/worker_test.go index 23467d69b39..344d2bcf605 100644 --- a/pkg/registry/apis/secret/garbagecollectionworker/worker_test.go +++ b/pkg/registry/apis/secret/garbagecollectionworker/worker_test.go @@ -9,7 +9,6 @@ import ( secretv1beta1 "github.com/grafana/grafana/apps/secret/pkg/apis/secret/v1beta1" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/registry/apis/secret/testutils" - "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" "github.com/grafana/grafana/pkg/storage/secret/encryption" "github.com/mitchellh/copystructure" "github.com/stretchr/testify/require" @@ -59,7 +58,7 @@ func TestBasic(t *testing.T) { require.NoError(t, err) // Get the secret value once to make sure it's reachable - exposedValue, err := keeper.Expose(t.Context(), keeperCfg, xkube.Namespace(sv.Namespace), sv.Name, sv.Status.Version) + exposedValue, err := keeper.Expose(t.Context(), keeperCfg, sv.Namespace, sv.Name, sv.Status.Version) require.NoError(t, err) require.NotEmpty(t, exposedValue.DangerouslyExposeAndConsumeValue()) @@ -79,7 +78,7 @@ func TestBasic(t *testing.T) { require.Empty(t, svs) // Try to get the secreet value again to make sure it's been deleted from the keeper - exposedValue, err = keeper.Expose(t.Context(), keeperCfg, xkube.Namespace(sv.Namespace), sv.Name, sv.Status.Version) + exposedValue, err = keeper.Expose(t.Context(), keeperCfg, sv.Namespace, sv.Name, sv.Status.Version) require.ErrorIs(t, err, encryption.ErrEncryptedValueNotFound) require.Empty(t, exposedValue) }) diff --git a/pkg/registry/apis/secret/secretkeeper/secretkeeper.go b/pkg/registry/apis/secret/secretkeeper/secretkeeper.go index dc51d5b3160..fc65573ac61 100644 --- a/pkg/registry/apis/secret/secretkeeper/secretkeeper.go +++ b/pkg/registry/apis/secret/secretkeeper/secretkeeper.go @@ -1,12 +1,9 @@ package secretkeeper import ( - "fmt" - "go.opentelemetry.io/otel/trace" secretv1beta1 "github.com/grafana/grafana/apps/secret/pkg/apis/secret/v1beta1" - "github.com/grafana/grafana/pkg/registry/apis/secret" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/registry/apis/secret/secretkeeper/sqlkeeper" "github.com/prometheus/client_golang/prometheus" @@ -23,17 +20,11 @@ func ProvideService( tracer trace.Tracer, store contracts.EncryptedValueStorage, encryptionManager contracts.EncryptionManager, - migrationExecutor contracts.EncryptedValueMigrationExecutor, reg prometheus.Registerer, - _ *secret.DependencyRegisterer, // noop import so wire runs DB migrations before instantiating this service -- can be nil when manually instantiating ) (*OSSKeeperService, error) { - systemKeeper, err := sqlkeeper.NewSQLKeeper(tracer, encryptionManager, store, migrationExecutor, reg) - if err != nil { - return nil, fmt.Errorf("failed to create system keeper: %w", err) - } - return &OSSKeeperService{ - systemKeeper: systemKeeper, + // TODO: rename to system keeper or something like that + systemKeeper: sqlkeeper.NewSQLKeeper(tracer, encryptionManager, store, reg), }, nil } diff --git a/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go b/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go index 43193befcea..84baaf207b0 100644 --- a/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go +++ b/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go @@ -12,7 +12,6 @@ import ( osskmsproviders "github.com/grafana/grafana/pkg/registry/apis/secret/encryption/kmsproviders" "github.com/grafana/grafana/pkg/registry/apis/secret/encryption/manager" "github.com/grafana/grafana/pkg/registry/apis/secret/secretkeeper/sqlkeeper" - "github.com/grafana/grafana/pkg/registry/apis/secret/testutils" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/storage/secret/database" @@ -66,8 +65,7 @@ func setupTestService(t *testing.T, cfg *setting.Cfg) (*OSSKeeperService, error) require.NoError(t, err) // Initialize the keeper service - keeperService, err := ProvideService(tracer, encValueStore, encryptionManager, &testutils.NoopMigrationExecutor{}, nil, nil) - require.NoError(t, err) + keeperService, err := ProvideService(tracer, encValueStore, encryptionManager, nil) return keeperService, err } diff --git a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go index a6f54d5674a..bfe3972f72b 100644 --- a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go +++ b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go @@ -5,11 +5,9 @@ import ( "fmt" "time" - "github.com/grafana/grafana-app-sdk/logging" secretv1beta1 "github.com/grafana/grafana/apps/secret/pkg/apis/secret/v1beta1" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/registry/apis/secret/secretkeeper/metrics" - "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -28,32 +26,20 @@ func NewSQLKeeper( tracer trace.Tracer, encryptionManager contracts.EncryptionManager, store contracts.EncryptedValueStorage, - migrationExecutor contracts.EncryptedValueMigrationExecutor, reg prometheus.Registerer, -) (*SQLKeeper, error) { - // Run the encrypted value store migration before anything else, otherwise operations may fail - // TODO: This does not need to be here forever, but we may currently have on-prem deployments using GSM, so it needs to be here for now. - // Periodically assess whether it is safe to remove - most likely for G13 should be fine. - log := logging.FromContext(context.Background()) - log.Debug("sqlkeeper: executing encrypted value store migration") - rowsAffected, err := migrationExecutor.Execute(context.Background()) - log.Debug("sqlkeeper: encrypted value store migration completed", "rows_affected", rowsAffected) - if err != nil { - return nil, fmt.Errorf("error encountered during encrypted value store migration: %w", err) - } - +) *SQLKeeper { return &SQLKeeper{ tracer: tracer, encryptionManager: encryptionManager, store: store, metrics: metrics.NewKeeperMetrics(reg), - }, nil + } } -func (s *SQLKeeper) Store(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace xkube.Namespace, name string, version int64, exposedValueOrRef string) (contracts.ExternalID, error) { +func (s *SQLKeeper) Store(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace, name string, version int64, exposedValueOrRef string) (contracts.ExternalID, error) { ctx, span := s.tracer.Start(ctx, "SQLKeeper.Store", trace.WithAttributes( - attribute.String("namespace", namespace.String()), + attribute.String("namespace", namespace), attribute.String("name", name), attribute.Int64("version", version)), ) @@ -77,9 +63,9 @@ func (s *SQLKeeper) Store(ctx context.Context, cfg secretv1beta1.KeeperConfig, n return contracts.ExternalID(""), nil } -func (s *SQLKeeper) Expose(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace xkube.Namespace, name string, version int64) (secretv1beta1.ExposedSecureValue, error) { +func (s *SQLKeeper) Expose(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace, name string, version int64) (secretv1beta1.ExposedSecureValue, error) { ctx, span := s.tracer.Start(ctx, "SQLKeeper.Expose", trace.WithAttributes( - attribute.String("namespace", namespace.String()), + attribute.String("namespace", namespace), attribute.String("name", name), attribute.Int64("version", version), )) @@ -91,7 +77,7 @@ func (s *SQLKeeper) Expose(ctx context.Context, cfg secretv1beta1.KeeperConfig, return "", fmt.Errorf("unable to get encrypted value: %w", err) } - exposedBytes, err := s.encryptionManager.Decrypt(ctx, namespace, encryptedValue.EncryptedPayload) + exposedBytes, err := s.encryptionManager.Decrypt(ctx, namespace, encryptedValue.EncryptedData) if err != nil { return "", fmt.Errorf("unable to decrypt value: %w", err) } @@ -102,9 +88,9 @@ func (s *SQLKeeper) Expose(ctx context.Context, cfg secretv1beta1.KeeperConfig, return exposedValue, nil } -func (s *SQLKeeper) Delete(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace xkube.Namespace, name string, version int64) error { +func (s *SQLKeeper) Delete(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace, name string, version int64) error { ctx, span := s.tracer.Start(ctx, "SQLKeeper.Delete", trace.WithAttributes( - attribute.String("namespace", namespace.String()), + attribute.String("namespace", namespace), attribute.String("name", name), attribute.Int64("version", version), )) @@ -121,9 +107,9 @@ func (s *SQLKeeper) Delete(ctx context.Context, cfg secretv1beta1.KeeperConfig, return nil } -func (s *SQLKeeper) Update(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace xkube.Namespace, name string, version int64, exposedValueOrRef string) error { +func (s *SQLKeeper) Update(ctx context.Context, cfg secretv1beta1.KeeperConfig, namespace, name string, version int64, exposedValueOrRef string) error { ctx, span := s.tracer.Start(ctx, "SQLKeeper.Update", trace.WithAttributes( - attribute.String("namespace", namespace.String()), + attribute.String("namespace", namespace), attribute.String("name", name), attribute.Int64("version", version), )) diff --git a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go index db5139c50ff..1333cf894c7 100644 --- a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go +++ b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go @@ -8,7 +8,6 @@ import ( secretv1beta1 "github.com/grafana/grafana/apps/secret/pkg/apis/secret/v1beta1" "github.com/grafana/grafana/pkg/registry/apis/secret/testutils" - "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" "github.com/grafana/grafana/pkg/tests/testsuite" ) @@ -17,10 +16,10 @@ func TestMain(m *testing.M) { } func Test_SQLKeeperSetup(t *testing.T) { - namespace1 := xkube.Namespace("namespace1") + namespace1 := "namespace1" name1 := "name1" version1 := int64(1) - namespace2 := xkube.Namespace("namespace2") + namespace2 := "namespace2" name2 := "name2" plaintext1 := "very secret string in namespace 1" plaintext2 := "very secret string in namespace 2" diff --git a/pkg/registry/apis/secret/service/consolidation.go b/pkg/registry/apis/secret/service/consolidation.go index bbe7d9084e6..b0baea659c4 100644 --- a/pkg/registry/apis/secret/service/consolidation.go +++ b/pkg/registry/apis/secret/service/consolidation.go @@ -6,7 +6,6 @@ import ( "github.com/grafana/grafana-app-sdk/logging" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" - "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" otelcodes "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" ) @@ -61,21 +60,21 @@ func (s *ConsolidationService) Consolidate(ctx context.Context) (err error) { for _, ev := range encryptedValues { // Decrypt the value using its old data key. - decryptedValue, err := s.encryptionManager.Decrypt(ctx, xkube.Namespace(ev.Namespace), ev.EncryptedPayload) + decryptedValue, err := s.encryptionManager.Decrypt(ctx, ev.Namespace, ev.EncryptedData) if err != nil { logging.FromContext(ctx).Error("Failed to decrypt value", "namespace", ev.Namespace, "name", ev.Name, "error", err) continue } // Re-encrypt the value using a new data key. - reEncryptedValue, err := s.encryptionManager.Encrypt(ctx, xkube.Namespace(ev.Namespace), decryptedValue) + reEncryptedValue, err := s.encryptionManager.Encrypt(ctx, ev.Namespace, decryptedValue) if err != nil { logging.FromContext(ctx).Error("Failed to re-encrypt value", "namespace", ev.Namespace, "name", ev.Name, "error", err) continue } // Update the encrypted value in the store. - err = s.encryptedValueStore.Update(ctx, xkube.Namespace(ev.Namespace), ev.Name, ev.Version, reEncryptedValue) + err = s.encryptedValueStore.Update(ctx, ev.Namespace, ev.Name, ev.Version, reEncryptedValue) if err != nil { logging.FromContext(ctx).Error("Failed to update encrypted value", "namespace", ev.Namespace, "name", ev.Name, "error", err) continue diff --git a/pkg/registry/apis/secret/service/consolidation_test.go b/pkg/registry/apis/secret/service/consolidation_test.go index 381cf956436..638ced19a81 100644 --- a/pkg/registry/apis/secret/service/consolidation_test.go +++ b/pkg/registry/apis/secret/service/consolidation_test.go @@ -97,7 +97,7 @@ func TestConsolidation(t *testing.T) { require.NoError(t, err) originalDecryptedValues = append(originalDecryptedValues, decryptedValue.DangerouslyExposeAndConsumeValue()) - encryptedValue, err := sut.EncryptedValueStorage.Get(ctx, xkube.Namespace(tc.namespace), tc.name, 1) + encryptedValue, err := sut.EncryptedValueStorage.Get(ctx, tc.namespace, tc.name, 1) require.NoError(t, err) require.NotNil(t, encryptedValue) originalEncryptedData = append(originalEncryptedData, encryptedValue.EncryptedData) @@ -115,7 +115,7 @@ func TestConsolidation(t *testing.T) { require.Equal(t, originalDecryptedValues[i], decryptedValue.DangerouslyExposeAndConsumeValue()) // Verify that the encrypted data has changed (indicating re-encryption) - encryptedValue, err := sut.EncryptedValueStorage.Get(ctx, xkube.Namespace(tc.namespace), tc.name, 1) + encryptedValue, err := sut.EncryptedValueStorage.Get(ctx, tc.namespace, tc.name, 1) require.NoError(t, err) require.NotEqual(t, originalEncryptedData[i], encryptedValue.EncryptedData) } @@ -174,7 +174,7 @@ func TestConsolidation(t *testing.T) { require.NoError(t, err) initialDecryptedValues = append(initialDecryptedValues, decryptedValue.DangerouslyExposeAndConsumeValue()) - encryptedValue, err := sut.EncryptedValueStorage.Get(ctx, xkube.Namespace(tc.namespace), tc.name, 1) + encryptedValue, err := sut.EncryptedValueStorage.Get(ctx, tc.namespace, tc.name, 1) require.NoError(t, err) initialEncryptedData = append(initialEncryptedData, encryptedValue.EncryptedData) } @@ -223,7 +223,7 @@ func TestConsolidation(t *testing.T) { require.NoError(t, err) newSecretDecryptedValues = append(newSecretDecryptedValues, decryptedValue.DangerouslyExposeAndConsumeValue()) - encryptedValue, err := sut.EncryptedValueStorage.Get(ctx, xkube.Namespace(tc.namespace), tc.name, 1) + encryptedValue, err := sut.EncryptedValueStorage.Get(ctx, tc.namespace, tc.name, 1) require.NoError(t, err) newSecretEncryptedData = append(newSecretEncryptedData, encryptedValue.EncryptedData) } @@ -252,7 +252,7 @@ func TestConsolidation(t *testing.T) { require.Equal(t, initialDecryptedValues[i], decryptedValue.DangerouslyExposeAndConsumeValue()) // Verify that the encrypted data has changed (indicating re-encryption) - encryptedValue, err := sut.EncryptedValueStorage.Get(ctx, xkube.Namespace(tc.namespace), tc.name, 1) + encryptedValue, err := sut.EncryptedValueStorage.Get(ctx, tc.namespace, tc.name, 1) require.NoError(t, err) require.NotEqual(t, initialEncryptedData[i], encryptedValue.EncryptedData) } @@ -275,7 +275,7 @@ func TestConsolidation(t *testing.T) { // Verify that the encrypted data has changed from what it was when first created // (indicating it was re-encrypted during consolidation) - encryptedValue, err := sut.EncryptedValueStorage.Get(ctx, xkube.Namespace(tc.namespace), tc.name, 1) + encryptedValue, err := sut.EncryptedValueStorage.Get(ctx, tc.namespace, tc.name, 1) require.NoError(t, err) require.NotEqual(t, newSecretEncryptedData[i], encryptedValue.EncryptedData) } diff --git a/pkg/registry/apis/secret/service/secure_value.go b/pkg/registry/apis/secret/service/secure_value.go index cc4e6c80c72..2ad69b759db 100644 --- a/pkg/registry/apis/secret/service/secure_value.go +++ b/pkg/registry/apis/secret/service/secure_value.go @@ -146,7 +146,7 @@ func (s *SecureValueService) Update(ctx context.Context, newSecureValue *secretv } logging.FromContext(ctx).Debug("retrieved keeper", "namespace", newSecureValue.Namespace, "keeperName", newSecureValue.Spec.Keeper, "type", keeperCfg.Type()) - secret, err := keeper.Expose(ctx, keeperCfg, xkube.Namespace(newSecureValue.Namespace), newSecureValue.Name, currentVersion.Status.Version) + secret, err := keeper.Expose(ctx, keeperCfg, newSecureValue.Namespace, newSecureValue.Name, currentVersion.Status.Version) if err != nil { return nil, false, fmt.Errorf("reading secret value from keeper: %w", err) } @@ -191,7 +191,7 @@ func (s *SecureValueService) createNewVersion(ctx context.Context, sv *secretv1b // TODO: can we stop using external id? // TODO: store uses only the namespace and returns and id. It could be a kv instead. // TODO: check that the encrypted store works with multiple versions - externalID, err := keeper.Store(ctx, keeperCfg, xkube.Namespace(createdSv.Namespace), createdSv.Name, createdSv.Status.Version, sv.Spec.Value.DangerouslyExposeAndConsumeValue()) + externalID, err := keeper.Store(ctx, keeperCfg, createdSv.Namespace, createdSv.Name, createdSv.Status.Version, sv.Spec.Value.DangerouslyExposeAndConsumeValue()) if err != nil { return nil, fmt.Errorf("storing secure value in keeper: %w", err) } diff --git a/pkg/registry/apis/secret/testutils/testutils.go b/pkg/registry/apis/secret/testutils/testutils.go index 2d1c94c2ea3..50190dbb3ff 100644 --- a/pkg/registry/apis/secret/testutils/testutils.go +++ b/pkg/registry/apis/secret/testutils/testutils.go @@ -126,14 +126,7 @@ func Setup(t *testing.T, opts ...func(*SetupConfig)) Sut { globalEncryptedValueStorage, err := encryptionstorage.ProvideGlobalEncryptedValueStorage(database, tracer) require.NoError(t, err) - // Initialize a noop migration executor for the sql keeper so it doesn't interfere with initialization - noopMigrationExecutor := &NoopMigrationExecutor{} - sqlKeeper, err := sqlkeeper.NewSQLKeeper(tracer, encryptionManager, encryptedValueStorage, noopMigrationExecutor, nil) - require.NoError(t, err) - - // Initialize a real migration executor for test - realMigrationExecutor, err := encryptionstorage.ProvideEncryptedValueMigrationExecutor(database, tracer, encryptedValueStorage, globalEncryptedValueStorage) - require.NoError(t, err) + sqlKeeper := sqlkeeper.NewSQLKeeper(tracer, encryptionManager, encryptedValueStorage, nil) var keeperService contracts.KeeperService = newKeeperServiceWrapper(sqlKeeper) @@ -165,41 +158,39 @@ func Setup(t *testing.T, opts ...func(*SetupConfig)) Sut { keeperService) return Sut{ - SecureValueService: secureValueService, - SecureValueMetadataStorage: secureValueMetadataStorage, - DecryptStorage: decryptStorage, - DecryptService: decryptService, - EncryptedValueStorage: encryptedValueStorage, - GlobalEncryptedValueStorage: globalEncryptedValueStorage, - EncryptedValueMigrationExecutor: realMigrationExecutor, - SQLKeeper: sqlKeeper, - Database: database, - AccessClient: accessClient, - ConsolidationService: consolidationService, - EncryptionManager: encryptionManager, - GlobalDataKeyStore: globalDataKeyStore, - GarbageCollectionWorker: garbageCollectionWorker, - Clock: clock, - KeeperService: keeperService, - KeeperMetadataStorage: keeperMetadataStorage, + SecureValueService: secureValueService, + SecureValueMetadataStorage: secureValueMetadataStorage, + DecryptStorage: decryptStorage, + DecryptService: decryptService, + EncryptedValueStorage: encryptedValueStorage, + GlobalEncryptedValueStorage: globalEncryptedValueStorage, + SQLKeeper: sqlKeeper, + Database: database, + AccessClient: accessClient, + ConsolidationService: consolidationService, + EncryptionManager: encryptionManager, + GlobalDataKeyStore: globalDataKeyStore, + GarbageCollectionWorker: garbageCollectionWorker, + Clock: clock, + KeeperService: keeperService, + KeeperMetadataStorage: keeperMetadataStorage, } } type Sut struct { - SecureValueService contracts.SecureValueService - SecureValueMetadataStorage contracts.SecureValueMetadataStorage - DecryptStorage contracts.DecryptStorage - DecryptService decryptcontracts.DecryptService - EncryptedValueStorage contracts.EncryptedValueStorage - GlobalEncryptedValueStorage contracts.GlobalEncryptedValueStorage - EncryptedValueMigrationExecutor contracts.EncryptedValueMigrationExecutor - SQLKeeper *sqlkeeper.SQLKeeper - Database *database.Database - AccessClient types.AccessClient - ConsolidationService contracts.ConsolidationService - EncryptionManager contracts.EncryptionManager - GlobalDataKeyStore contracts.GlobalDataKeyStorage - GarbageCollectionWorker *garbagecollectionworker.Worker + SecureValueService contracts.SecureValueService + SecureValueMetadataStorage contracts.SecureValueMetadataStorage + DecryptStorage contracts.DecryptStorage + DecryptService decryptcontracts.DecryptService + EncryptedValueStorage contracts.EncryptedValueStorage + GlobalEncryptedValueStorage contracts.GlobalEncryptedValueStorage + SQLKeeper *sqlkeeper.SQLKeeper + Database *database.Database + AccessClient types.AccessClient + ConsolidationService contracts.ConsolidationService + EncryptionManager contracts.EncryptionManager + GlobalDataKeyStore contracts.GlobalDataKeyStorage + GarbageCollectionWorker *garbagecollectionworker.Worker // The fake clock passed to implementations to make testing easier Clock *FakeClock KeeperService contracts.KeeperService @@ -375,10 +366,3 @@ func (c *FakeClock) Now() time.Time { func (c *FakeClock) AdvanceBy(duration time.Duration) { c.Current = c.Current.Add(duration) } - -type NoopMigrationExecutor struct { -} - -func (e *NoopMigrationExecutor) Execute(ctx context.Context) (int, error) { - return 0, nil -} diff --git a/pkg/server/wire.go b/pkg/server/wire.go index 5422ac453ae..9ee5cfeaf68 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -444,7 +444,6 @@ var wireBasicSet = wire.NewSet( secretencryption.ProvideGlobalDataKeyStorage, secretencryption.ProvideEncryptedValueStorage, secretencryption.ProvideGlobalEncryptedValueStorage, - secretencryption.ProvideEncryptedValueMigrationExecutor, secretsecurevalueservice.ProvideSecureValueService, secretvalidator.ProvideKeeperValidator, secretvalidator.ProvideSecureValueValidator, diff --git a/pkg/server/wire_gen.go b/pkg/server/wire_gen.go index eba2e7b9ff5..ee58cbe1d21 100644 --- a/pkg/server/wire_gen.go +++ b/pkg/server/wire_gen.go @@ -217,7 +217,7 @@ import ( kvstore2 "github.com/grafana/grafana/pkg/services/secrets/kvstore" migrations2 "github.com/grafana/grafana/pkg/services/secrets/kvstore/migrations" "github.com/grafana/grafana/pkg/services/secrets/manager" - migrator2 "github.com/grafana/grafana/pkg/services/secrets/migrator" + "github.com/grafana/grafana/pkg/services/secrets/migrator" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/extsvcaccounts" manager3 "github.com/grafana/grafana/pkg/services/serviceaccounts/manager" @@ -255,7 +255,7 @@ import ( database4 "github.com/grafana/grafana/pkg/storage/secret/database" "github.com/grafana/grafana/pkg/storage/secret/encryption" "github.com/grafana/grafana/pkg/storage/secret/metadata" - "github.com/grafana/grafana/pkg/storage/secret/migrator" + migrator2 "github.com/grafana/grafana/pkg/storage/secret/migrator" "github.com/grafana/grafana/pkg/storage/unified" "github.com/grafana/grafana/pkg/storage/unified/resource" "github.com/grafana/grafana/pkg/storage/unified/search" @@ -482,20 +482,7 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api if err != nil { return nil, err } - globalEncryptedValueStorage, err := encryption.ProvideGlobalEncryptedValueStorage(databaseDatabase, tracer) - if err != nil { - return nil, err - } - encryptedValueMigrationExecutor, err := encryption.ProvideEncryptedValueMigrationExecutor(databaseDatabase, tracer, encryptedValueStorage, globalEncryptedValueStorage) - if err != nil { - return nil, err - } - secretDBMigrator := migrator.NewWithEngine(sqlStore) - dependencyRegisterer, err := secret.RegisterDependencies(featureToggles, cfg, secretDBMigrator, acimplService) - if err != nil { - return nil, err - } - ossKeeperService, err := secretkeeper.ProvideService(tracer, encryptedValueStorage, encryptionManager, encryptedValueMigrationExecutor, registerer, dependencyRegisterer) + ossKeeperService, err := secretkeeper.ProvideService(tracer, encryptedValueStorage, encryptionManager, registerer) if err != nil { return nil, err } @@ -700,7 +687,7 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api } csrfCSRF := csrf.ProvideCSRFFilter(cfg) playlistService := playlistimpl.ProvideService(sqlStore, tracingService) - secretsMigrator := migrator2.ProvideSecretsMigrator(serviceService, secretsService, sqlStore, ossImpl, featureToggles) + secretsMigrator := migrator.ProvideSecretsMigrator(serviceService, secretsService, sqlStore, ossImpl, featureToggles) dataSourceSecretMigrationService := migrations2.ProvideDataSourceMigrationService(service15, kvStore, featureToggles) secretMigrationProviderImpl := migrations2.ProvideSecretMigrationProvider(serverLockService, dataSourceSecretMigrationService) publicDashboardServiceImpl := service3.ProvideService(cfg, featureToggles, publicDashboardStoreImpl, queryServiceImpl, repositoryImpl, accessControl, publicDashboardServiceWrapperImpl, dashboardService, ossLicensingService) @@ -871,6 +858,11 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api if err != nil { return nil, err } + secretDBMigrator := migrator2.NewWithEngine(sqlStore) + dependencyRegisterer, err := secret.RegisterDependencies(featureToggles, cfg, secretDBMigrator, acimplService) + if err != nil { + return nil, err + } apiregistryService := apiregistry.ProvideRegistryServiceSink(dashboardsAPIBuilder, snapshotsAPIBuilder, dataSourceAPIBuilder, folderAPIBuilder, identityAccessManagementAPIBuilder, queryAPIBuilder, userStorageAPIBuilder, apiBuilder, provisioningAPIBuilder, ofrepAPIBuilder, dependencyRegisterer) teamPermissionsService, err := ossaccesscontrol.ProvideTeamPermissions(cfg, featureToggles, routeRegisterImpl, sqlStore, accessControl, ossLicensingService, acimplService, teamService, userService, actionSetService) if err != nil { @@ -1096,20 +1088,7 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac if err != nil { return nil, err } - globalEncryptedValueStorage, err := encryption.ProvideGlobalEncryptedValueStorage(databaseDatabase, tracer) - if err != nil { - return nil, err - } - encryptedValueMigrationExecutor, err := encryption.ProvideEncryptedValueMigrationExecutor(databaseDatabase, tracer, encryptedValueStorage, globalEncryptedValueStorage) - if err != nil { - return nil, err - } - secretDBMigrator := migrator.NewWithEngine(sqlStore) - dependencyRegisterer, err := secret.RegisterDependencies(featureToggles, cfg, secretDBMigrator, acimplService) - if err != nil { - return nil, err - } - ossKeeperService, err := secretkeeper.ProvideService(tracer, encryptedValueStorage, encryptionManager, encryptedValueMigrationExecutor, registerer, dependencyRegisterer) + ossKeeperService, err := secretkeeper.ProvideService(tracer, encryptedValueStorage, encryptionManager, registerer) if err != nil { return nil, err } @@ -1316,7 +1295,7 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac } csrfCSRF := csrf.ProvideCSRFFilter(cfg) playlistService := playlistimpl.ProvideService(sqlStore, tracingService) - secretsMigrator := migrator2.ProvideSecretsMigrator(serviceService, secretsService, sqlStore, ossImpl, featureToggles) + secretsMigrator := migrator.ProvideSecretsMigrator(serviceService, secretsService, sqlStore, ossImpl, featureToggles) dataSourceSecretMigrationService := migrations2.ProvideDataSourceMigrationService(service15, kvStore, featureToggles) secretMigrationProviderImpl := migrations2.ProvideSecretMigrationProvider(serverLockService, dataSourceSecretMigrationService) publicDashboardServiceImpl := service3.ProvideService(cfg, featureToggles, publicDashboardStoreImpl, queryServiceImpl, repositoryImpl, accessControl, publicDashboardServiceWrapperImpl, dashboardService, ossLicensingService) @@ -1487,6 +1466,11 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac if err != nil { return nil, err } + secretDBMigrator := migrator2.NewWithEngine(sqlStore) + dependencyRegisterer, err := secret.RegisterDependencies(featureToggles, cfg, secretDBMigrator, acimplService) + if err != nil { + return nil, err + } apiregistryService := apiregistry.ProvideRegistryServiceSink(dashboardsAPIBuilder, snapshotsAPIBuilder, dataSourceAPIBuilder, folderAPIBuilder, identityAccessManagementAPIBuilder, queryAPIBuilder, userStorageAPIBuilder, apiBuilder, provisioningAPIBuilder, ofrepAPIBuilder, dependencyRegisterer) teamPermissionsService, err := ossaccesscontrol.ProvideTeamPermissions(cfg, featureToggles, routeRegisterImpl, sqlStore, accessControl, ossLicensingService, acimplService, teamService, userService, actionSetService) if err != nil { @@ -1556,7 +1540,7 @@ func InitializeForCLI(ctx context.Context, cfg *setting.Cfg) (Runner, error) { if err != nil { return Runner{}, err } - secretsMigrator := migrator2.ProvideSecretsMigrator(serviceService, secretsService, sqlStore, ossImpl, featureToggles) + secretsMigrator := migrator.ProvideSecretsMigrator(serviceService, secretsService, sqlStore, ossImpl, featureToggles) configProvider, err := configprovider.ProvideService(cfg) if err != nil { return Runner{}, err @@ -1694,7 +1678,7 @@ var withOTelSet = wire.NewSet( otelTracer, grpcserver.ProvideService, interceptors.ProvideAuthenticator, ) -var wireBasicSet = wire.NewSet(annotationsimpl.ProvideService, wire.Bind(new(annotations.Repository), new(*annotationsimpl.RepositoryImpl)), New, api.ProvideHTTPServer, query.ProvideService, wire.Bind(new(query.Service), new(*query.ServiceImpl)), bus.ProvideBus, wire.Bind(new(bus.Bus), new(*bus.InProcBus)), rendering.ProvideService, wire.Bind(new(rendering.Service), new(*rendering.RenderingService)), routing.ProvideRegister, wire.Bind(new(routing.RouteRegister), new(*routing.RouteRegisterImpl)), hooks.ProvideService, kvstore.ProvideService, localcache.ProvideService, bundleregistry.ProvideService, wire.Bind(new(supportbundles.Service), new(*bundleregistry.Service)), updatemanager.ProvideGrafanaService, updatemanager.ProvidePluginsService, service.ProvideService, wire.Bind(new(usagestats.Service), new(*service.UsageStats)), validator3.ProvideService, legacy.ProvideLegacyMigrator, pluginsintegration.WireSet, dashboards.ProvideFileStoreManager, wire.Bind(new(dashboards.FileStore), new(*dashboards.FileStoreManager)), cloudwatch.ProvideService, cloudmonitoring.ProvideService, azuremonitor.ProvideService, postgres.ProvideService, mysql.ProvideService, mssql.ProvideService, store.ProvideEntityEventsService, dualwrite.ProvideService, httpclientprovider.New, wire.Bind(new(httpclient.Provider), new(*httpclient2.Provider)), serverlock.ProvideService, annotationsimpl.ProvideCleanupService, wire.Bind(new(annotations.Cleaner), new(*annotationsimpl.CleanupServiceImpl)), cleanup.ProvideService, shorturlimpl.ProvideService, wire.Bind(new(shorturls.Service), new(*shorturlimpl.ShortURLService)), queryhistory.ProvideService, wire.Bind(new(queryhistory.Service), new(*queryhistory.QueryHistoryService)), correlations.ProvideService, wire.Bind(new(correlations.Service), new(*correlations.CorrelationsService)), quotaimpl.ProvideService, remotecache.ProvideService, wire.Bind(new(remotecache.CacheStorage), new(*remotecache.RemoteCache)), authinfoimpl.ProvideService, wire.Bind(new(login.AuthInfoService), new(*authinfoimpl.Service)), authinfoimpl.ProvideStore, datasourceproxy.ProvideService, sort.ProvideService, search2.ProvideService, searchV2.ProvideService, searchV2.ProvideSearchHTTPService, store.ProvideService, store.ProvideSystemUsersService, live.ProvideService, pushhttp.ProvideService, contexthandler.ProvideService, service12.ProvideService, wire.Bind(new(service12.LDAP), new(*service12.LDAPImpl)), jwt.ProvideService, wire.Bind(new(jwt.JWTService), new(*jwt.AuthService)), store2.ProvideDBStore, image.ProvideDeleteExpiredService, ngalert.ProvideService, librarypanels.ProvideService, wire.Bind(new(librarypanels.Service), new(*librarypanels.LibraryPanelService)), libraryelements.ProvideService, wire.Bind(new(libraryelements.Service), new(*libraryelements.LibraryElementService)), notifications.ProvideService, notifications.ProvideSmtpService, github.ProvideFactory, tracing.ProvideService, tracing.ProvideTracingConfig, wire.Bind(new(tracing.Tracer), new(*tracing.TracingService)), withOTelSet, testdatasource.ProvideService, api4.ProvideService, opentsdb.ProvideService, socialimpl.ProvideService, influxdb.ProvideService, wire.Bind(new(social.Service), new(*socialimpl.SocialService)), tempo.ProvideService, loki.ProvideService, graphite.ProvideService, prometheus.ProvideService, elasticsearch.ProvideService, pyroscope.ProvideService, parca.ProvideService, zipkin.ProvideService, jaeger.ProvideService, service9.ProvideCacheService, wire.Bind(new(datasources.CacheService), new(*service9.CacheServiceImpl)), service2.ProvideEncryptionService, wire.Bind(new(encryption2.Internal), new(*service2.Service)), manager.ProvideSecretsService, wire.Bind(new(secrets.Service), new(*manager.SecretsService)), database.ProvideSecretsStore, wire.Bind(new(secrets.Store), new(*database.SecretsStoreImpl)), garbagecollectionworker.ProvideWorker, grafanads.ProvideService, wire.Bind(new(dashboardsnapshots.Store), new(*database5.DashboardSnapshotStore)), database5.ProvideStore, wire.Bind(new(dashboardsnapshots.Service), new(*service10.ServiceImpl)), service10.ProvideService, service9.ProvideService, wire.Bind(new(datasources.DataSourceService), new(*service9.Service)), service9.ProvideLegacyDataSourceLookup, retriever.ProvideService, wire.Bind(new(serviceaccounts.ServiceAccountRetriever), new(*retriever.Service)), ossaccesscontrol.ProvideServiceAccountPermissions, wire.Bind(new(accesscontrol.ServiceAccountPermissionsService), new(*ossaccesscontrol.ServiceAccountPermissionsService)), manager3.ProvideServiceAccountsService, proxy.ProvideServiceAccountsProxy, wire.Bind(new(serviceaccounts.Service), new(*proxy.ServiceAccountsProxy)), dsquerierclient.NewNullQSDatasourceClientBuilder, expr.ProvideService, featuremgmt.ProvideManagerService, featuremgmt.ProvideToggles, service7.ProvideDashboardServiceImpl, wire.Bind(new(dashboards2.PermissionsRegistrationService), new(*service7.DashboardServiceImpl)), service7.ProvideDashboardService, service7.ProvideDashboardProvisioningService, service7.ProvideDashboardPluginService, database2.ProvideDashboardStore, folderimpl.ProvideService, wire.Bind(new(folder.Service), new(*folderimpl.Service)), wire.Bind(new(folder.LegacyService), new(*folderimpl.Service)), folderimpl.ProvideStore, wire.Bind(new(folder.Store), new(*folderimpl.FolderStoreImpl)), service11.ProvideService, wire.Bind(new(dashboardimport.Service), new(*service11.ImportDashboardService)), service8.ProvideService, wire.Bind(new(plugindashboards.Service), new(*service8.Service)), service8.ProvideDashboardUpdater, kvstore2.ProvideService, avatar.ProvideAvatarCacheServer, statscollector.ProvideService, csrf.ProvideCSRFFilter, wire.Bind(new(csrf.Service), new(*csrf.CSRF)), ossaccesscontrol.ProvideTeamPermissions, wire.Bind(new(accesscontrol.TeamPermissionsService), new(*ossaccesscontrol.TeamPermissionsService)), ossaccesscontrol.ProvideFolderPermissions, wire.Bind(new(accesscontrol.FolderPermissionsService), new(*ossaccesscontrol.FolderPermissionsService)), ossaccesscontrol.ProvideDashboardPermissions, wire.Bind(new(accesscontrol.DashboardPermissionsService), new(*ossaccesscontrol.DashboardPermissionsService)), ossaccesscontrol.ProvideReceiverPermissionsService, wire.Bind(new(accesscontrol.ReceiverPermissionsService), new(*ossaccesscontrol.ReceiverPermissionsService)), starimpl.ProvideService, playlistimpl.ProvideService, apikeyimpl.ProvideService, dashverimpl.ProvideService, service3.ProvideService, wire.Bind(new(publicdashboards.Service), new(*service3.PublicDashboardServiceImpl)), database3.ProvideStore, wire.Bind(new(publicdashboards.Store), new(*database3.PublicDashboardStoreImpl)), metric.ProvideService, api2.ProvideApi, api3.ProvideApi, userimpl.ProvideService, orgimpl.ProvideService, orgimpl.ProvideDeletionService, statsimpl.ProvideService, grpccontext.ProvideContextHandler, grpcserver.ProvideHealthService, grpcserver.ProvideReflectionService, resolver.ProvideEntityReferenceResolver, teamimpl.ProvideService, teamapi.ProvideTeamAPI, tempuserimpl.ProvideService, loginattemptimpl.ProvideService, wire.Bind(new(loginattempt.Service), new(*loginattemptimpl.Service)), migrations2.ProvideDataSourceMigrationService, migrations2.ProvideSecretMigrationProvider, wire.Bind(new(migrations2.SecretMigrationProvider), new(*migrations2.SecretMigrationProviderImpl)), promtypemigration.ProvideAzurePromMigrationService, promtypemigration.ProvideAmazonPromMigrationService, promtypemigration.ProvidePromTypeMigrationProvider, wire.Bind(new(promtypemigration.PromTypeMigrationProvider), new(*promtypemigration.PromTypeMigrationProviderImpl)), resourcepermissions.NewActionSetService, wire.Bind(new(accesscontrol.ActionResolver), new(resourcepermissions.ActionSetService)), wire.Bind(new(pluginaccesscontrol.ActionSetRegistry), new(resourcepermissions.ActionSetService)), permreg.ProvidePermissionRegistry, acimpl.ProvideAccessControl, dualwrite2.ProvideZanzanaReconciler, navtreeimpl.ProvideService, wire.Bind(new(accesscontrol.AccessControl), new(*acimpl.AccessControl)), wire.Bind(new(notifications.TempUserStore), new(tempuser.Service)), tagimpl.ProvideService, wire.Bind(new(tag.Service), new(*tagimpl.Service)), authnimpl.ProvideService, authnimpl.ProvideIdentitySynchronizer, authnimpl.ProvideAuthnService, authnimpl.ProvideAuthnServiceAuthenticateOnly, authnimpl.ProvideRegistration, supportbundlesimpl.ProvideService, extsvcaccounts.ProvideExtSvcAccountsService, wire.Bind(new(serviceaccounts.ExtSvcAccountsService), new(*extsvcaccounts.ExtSvcAccountsService)), registry2.ProvideExtSvcRegistry, wire.Bind(new(extsvcauth.ExternalServiceRegistry), new(*registry2.Registry)), anonstore.ProvideAnonDBStore, wire.Bind(new(anonstore.AnonStore), new(*anonstore.AnonDBStore)), loggermw.Provide, slogadapter.Provide, signingkeysimpl.ProvideEmbeddedSigningKeysService, wire.Bind(new(signingkeys.Service), new(*signingkeysimpl.Service)), ssosettingsimpl.ProvideService, wire.Bind(new(ssosettings.Service), new(*ssosettingsimpl.Service)), idimpl.ProvideService, wire.Bind(new(auth.IDService), new(*idimpl.Service)), cloudmigrationimpl.ProvideService, userimpl.ProvideVerifier, connectors.ProvideOrgRoleMapper, wire.Bind(new(user.Verifier), new(*userimpl.Verifier)), authz.WireSet, metadata.ProvideSecureValueMetadataStorage, metadata.ProvideKeeperMetadataStorage, metadata.ProvideDecryptStorage, decrypt.ProvideDecryptAuthorizer, wire.Value([]decrypt.ExtraOwnerDecrypter(nil)), decrypt.ProvideDecryptService, inline.ProvideInlineSecureValueService, encryption.ProvideDataKeyStorage, encryption.ProvideGlobalDataKeyStorage, encryption.ProvideEncryptedValueStorage, encryption.ProvideGlobalEncryptedValueStorage, encryption.ProvideEncryptedValueMigrationExecutor, service5.ProvideSecureValueService, validator.ProvideKeeperValidator, validator.ProvideSecureValueValidator, mutator.ProvideKeeperMutator, mutator.ProvideSecureValueMutator, migrator.NewWithEngine, database4.ProvideDatabase, clock.ProvideClock, wire.Bind(new(contracts.Database), new(*database4.Database)), wire.Bind(new(contracts.Clock), new(*clock.Clock)), manager2.ProvideEncryptionManager, service4.ProvideAESGCMCipherService, resource.ProvideStorageMetrics, resource.ProvideIndexMetrics, apiserver.WireSet, apiregistry.WireSet, appregistry.WireSet, client.ProvideK8sClientWithFallback) +var wireBasicSet = wire.NewSet(annotationsimpl.ProvideService, wire.Bind(new(annotations.Repository), new(*annotationsimpl.RepositoryImpl)), New, api.ProvideHTTPServer, query.ProvideService, wire.Bind(new(query.Service), new(*query.ServiceImpl)), bus.ProvideBus, wire.Bind(new(bus.Bus), new(*bus.InProcBus)), rendering.ProvideService, wire.Bind(new(rendering.Service), new(*rendering.RenderingService)), routing.ProvideRegister, wire.Bind(new(routing.RouteRegister), new(*routing.RouteRegisterImpl)), hooks.ProvideService, kvstore.ProvideService, localcache.ProvideService, bundleregistry.ProvideService, wire.Bind(new(supportbundles.Service), new(*bundleregistry.Service)), updatemanager.ProvideGrafanaService, updatemanager.ProvidePluginsService, service.ProvideService, wire.Bind(new(usagestats.Service), new(*service.UsageStats)), validator3.ProvideService, legacy.ProvideLegacyMigrator, pluginsintegration.WireSet, dashboards.ProvideFileStoreManager, wire.Bind(new(dashboards.FileStore), new(*dashboards.FileStoreManager)), cloudwatch.ProvideService, cloudmonitoring.ProvideService, azuremonitor.ProvideService, postgres.ProvideService, mysql.ProvideService, mssql.ProvideService, store.ProvideEntityEventsService, dualwrite.ProvideService, httpclientprovider.New, wire.Bind(new(httpclient.Provider), new(*httpclient2.Provider)), serverlock.ProvideService, annotationsimpl.ProvideCleanupService, wire.Bind(new(annotations.Cleaner), new(*annotationsimpl.CleanupServiceImpl)), cleanup.ProvideService, shorturlimpl.ProvideService, wire.Bind(new(shorturls.Service), new(*shorturlimpl.ShortURLService)), queryhistory.ProvideService, wire.Bind(new(queryhistory.Service), new(*queryhistory.QueryHistoryService)), correlations.ProvideService, wire.Bind(new(correlations.Service), new(*correlations.CorrelationsService)), quotaimpl.ProvideService, remotecache.ProvideService, wire.Bind(new(remotecache.CacheStorage), new(*remotecache.RemoteCache)), authinfoimpl.ProvideService, wire.Bind(new(login.AuthInfoService), new(*authinfoimpl.Service)), authinfoimpl.ProvideStore, datasourceproxy.ProvideService, sort.ProvideService, search2.ProvideService, searchV2.ProvideService, searchV2.ProvideSearchHTTPService, store.ProvideService, store.ProvideSystemUsersService, live.ProvideService, pushhttp.ProvideService, contexthandler.ProvideService, service12.ProvideService, wire.Bind(new(service12.LDAP), new(*service12.LDAPImpl)), jwt.ProvideService, wire.Bind(new(jwt.JWTService), new(*jwt.AuthService)), store2.ProvideDBStore, image.ProvideDeleteExpiredService, ngalert.ProvideService, librarypanels.ProvideService, wire.Bind(new(librarypanels.Service), new(*librarypanels.LibraryPanelService)), libraryelements.ProvideService, wire.Bind(new(libraryelements.Service), new(*libraryelements.LibraryElementService)), notifications.ProvideService, notifications.ProvideSmtpService, github.ProvideFactory, tracing.ProvideService, tracing.ProvideTracingConfig, wire.Bind(new(tracing.Tracer), new(*tracing.TracingService)), withOTelSet, testdatasource.ProvideService, api4.ProvideService, opentsdb.ProvideService, socialimpl.ProvideService, influxdb.ProvideService, wire.Bind(new(social.Service), new(*socialimpl.SocialService)), tempo.ProvideService, loki.ProvideService, graphite.ProvideService, prometheus.ProvideService, elasticsearch.ProvideService, pyroscope.ProvideService, parca.ProvideService, zipkin.ProvideService, jaeger.ProvideService, service9.ProvideCacheService, wire.Bind(new(datasources.CacheService), new(*service9.CacheServiceImpl)), service2.ProvideEncryptionService, wire.Bind(new(encryption2.Internal), new(*service2.Service)), manager.ProvideSecretsService, wire.Bind(new(secrets.Service), new(*manager.SecretsService)), database.ProvideSecretsStore, wire.Bind(new(secrets.Store), new(*database.SecretsStoreImpl)), garbagecollectionworker.ProvideWorker, grafanads.ProvideService, wire.Bind(new(dashboardsnapshots.Store), new(*database5.DashboardSnapshotStore)), database5.ProvideStore, wire.Bind(new(dashboardsnapshots.Service), new(*service10.ServiceImpl)), service10.ProvideService, service9.ProvideService, wire.Bind(new(datasources.DataSourceService), new(*service9.Service)), service9.ProvideLegacyDataSourceLookup, retriever.ProvideService, wire.Bind(new(serviceaccounts.ServiceAccountRetriever), new(*retriever.Service)), ossaccesscontrol.ProvideServiceAccountPermissions, wire.Bind(new(accesscontrol.ServiceAccountPermissionsService), new(*ossaccesscontrol.ServiceAccountPermissionsService)), manager3.ProvideServiceAccountsService, proxy.ProvideServiceAccountsProxy, wire.Bind(new(serviceaccounts.Service), new(*proxy.ServiceAccountsProxy)), dsquerierclient.NewNullQSDatasourceClientBuilder, expr.ProvideService, featuremgmt.ProvideManagerService, featuremgmt.ProvideToggles, service7.ProvideDashboardServiceImpl, wire.Bind(new(dashboards2.PermissionsRegistrationService), new(*service7.DashboardServiceImpl)), service7.ProvideDashboardService, service7.ProvideDashboardProvisioningService, service7.ProvideDashboardPluginService, database2.ProvideDashboardStore, folderimpl.ProvideService, wire.Bind(new(folder.Service), new(*folderimpl.Service)), wire.Bind(new(folder.LegacyService), new(*folderimpl.Service)), folderimpl.ProvideStore, wire.Bind(new(folder.Store), new(*folderimpl.FolderStoreImpl)), service11.ProvideService, wire.Bind(new(dashboardimport.Service), new(*service11.ImportDashboardService)), service8.ProvideService, wire.Bind(new(plugindashboards.Service), new(*service8.Service)), service8.ProvideDashboardUpdater, kvstore2.ProvideService, avatar.ProvideAvatarCacheServer, statscollector.ProvideService, csrf.ProvideCSRFFilter, wire.Bind(new(csrf.Service), new(*csrf.CSRF)), ossaccesscontrol.ProvideTeamPermissions, wire.Bind(new(accesscontrol.TeamPermissionsService), new(*ossaccesscontrol.TeamPermissionsService)), ossaccesscontrol.ProvideFolderPermissions, wire.Bind(new(accesscontrol.FolderPermissionsService), new(*ossaccesscontrol.FolderPermissionsService)), ossaccesscontrol.ProvideDashboardPermissions, wire.Bind(new(accesscontrol.DashboardPermissionsService), new(*ossaccesscontrol.DashboardPermissionsService)), ossaccesscontrol.ProvideReceiverPermissionsService, wire.Bind(new(accesscontrol.ReceiverPermissionsService), new(*ossaccesscontrol.ReceiverPermissionsService)), starimpl.ProvideService, playlistimpl.ProvideService, apikeyimpl.ProvideService, dashverimpl.ProvideService, service3.ProvideService, wire.Bind(new(publicdashboards.Service), new(*service3.PublicDashboardServiceImpl)), database3.ProvideStore, wire.Bind(new(publicdashboards.Store), new(*database3.PublicDashboardStoreImpl)), metric.ProvideService, api2.ProvideApi, api3.ProvideApi, userimpl.ProvideService, orgimpl.ProvideService, orgimpl.ProvideDeletionService, statsimpl.ProvideService, grpccontext.ProvideContextHandler, grpcserver.ProvideHealthService, grpcserver.ProvideReflectionService, resolver.ProvideEntityReferenceResolver, teamimpl.ProvideService, teamapi.ProvideTeamAPI, tempuserimpl.ProvideService, loginattemptimpl.ProvideService, wire.Bind(new(loginattempt.Service), new(*loginattemptimpl.Service)), migrations2.ProvideDataSourceMigrationService, migrations2.ProvideSecretMigrationProvider, wire.Bind(new(migrations2.SecretMigrationProvider), new(*migrations2.SecretMigrationProviderImpl)), promtypemigration.ProvideAzurePromMigrationService, promtypemigration.ProvideAmazonPromMigrationService, promtypemigration.ProvidePromTypeMigrationProvider, wire.Bind(new(promtypemigration.PromTypeMigrationProvider), new(*promtypemigration.PromTypeMigrationProviderImpl)), resourcepermissions.NewActionSetService, wire.Bind(new(accesscontrol.ActionResolver), new(resourcepermissions.ActionSetService)), wire.Bind(new(pluginaccesscontrol.ActionSetRegistry), new(resourcepermissions.ActionSetService)), permreg.ProvidePermissionRegistry, acimpl.ProvideAccessControl, dualwrite2.ProvideZanzanaReconciler, navtreeimpl.ProvideService, wire.Bind(new(accesscontrol.AccessControl), new(*acimpl.AccessControl)), wire.Bind(new(notifications.TempUserStore), new(tempuser.Service)), tagimpl.ProvideService, wire.Bind(new(tag.Service), new(*tagimpl.Service)), authnimpl.ProvideService, authnimpl.ProvideIdentitySynchronizer, authnimpl.ProvideAuthnService, authnimpl.ProvideAuthnServiceAuthenticateOnly, authnimpl.ProvideRegistration, supportbundlesimpl.ProvideService, extsvcaccounts.ProvideExtSvcAccountsService, wire.Bind(new(serviceaccounts.ExtSvcAccountsService), new(*extsvcaccounts.ExtSvcAccountsService)), registry2.ProvideExtSvcRegistry, wire.Bind(new(extsvcauth.ExternalServiceRegistry), new(*registry2.Registry)), anonstore.ProvideAnonDBStore, wire.Bind(new(anonstore.AnonStore), new(*anonstore.AnonDBStore)), loggermw.Provide, slogadapter.Provide, signingkeysimpl.ProvideEmbeddedSigningKeysService, wire.Bind(new(signingkeys.Service), new(*signingkeysimpl.Service)), ssosettingsimpl.ProvideService, wire.Bind(new(ssosettings.Service), new(*ssosettingsimpl.Service)), idimpl.ProvideService, wire.Bind(new(auth.IDService), new(*idimpl.Service)), cloudmigrationimpl.ProvideService, userimpl.ProvideVerifier, connectors.ProvideOrgRoleMapper, wire.Bind(new(user.Verifier), new(*userimpl.Verifier)), authz.WireSet, metadata.ProvideSecureValueMetadataStorage, metadata.ProvideKeeperMetadataStorage, metadata.ProvideDecryptStorage, decrypt.ProvideDecryptAuthorizer, wire.Value([]decrypt.ExtraOwnerDecrypter(nil)), decrypt.ProvideDecryptService, inline.ProvideInlineSecureValueService, encryption.ProvideDataKeyStorage, encryption.ProvideGlobalDataKeyStorage, encryption.ProvideEncryptedValueStorage, encryption.ProvideGlobalEncryptedValueStorage, service5.ProvideSecureValueService, validator.ProvideKeeperValidator, validator.ProvideSecureValueValidator, mutator.ProvideKeeperMutator, mutator.ProvideSecureValueMutator, migrator2.NewWithEngine, database4.ProvideDatabase, clock.ProvideClock, wire.Bind(new(contracts.Database), new(*database4.Database)), wire.Bind(new(contracts.Clock), new(*clock.Clock)), manager2.ProvideEncryptionManager, service4.ProvideAESGCMCipherService, resource.ProvideStorageMetrics, resource.ProvideIndexMetrics, apiserver.WireSet, apiregistry.WireSet, appregistry.WireSet, client.ProvideK8sClientWithFallback) var wireSet = wire.NewSet( wireBasicSet, metrics.WireSet, sqlstore.ProvideService, metrics2.ProvideService, wire.Bind(new(notifications.Service), new(*notifications.NotificationService)), wire.Bind(new(notifications.WebhookSender), new(*notifications.NotificationService)), wire.Bind(new(notifications.EmailSender), new(*notifications.NotificationService)), wire.Bind(new(db.DB), new(*sqlstore.SQLStore)), prefimpl.ProvideService, oauthtoken.ProvideService, wire.Bind(new(oauthtoken.OAuthTokenService), new(*oauthtoken.Service)), wire.Bind(new(cleanup.AlertRuleService), new(*store2.DBstore)), diff --git a/pkg/storage/secret/encryption/data/encrypted_value_create.sql b/pkg/storage/secret/encryption/data/encrypted_value_create.sql index 7aa749e01a3..7bce6a19bd4 100644 --- a/pkg/storage/secret/encryption/data/encrypted_value_create.sql +++ b/pkg/storage/secret/encryption/data/encrypted_value_create.sql @@ -3,7 +3,6 @@ INSERT INTO {{ .Ident "secret_encrypted_value" }} ( {{ .Ident "name" }}, {{ .Ident "version" }}, {{ .Ident "encrypted_data" }}, - {{ .Ident "data_key_id" }}, {{ .Ident "created" }}, {{ .Ident "updated" }} ) VALUES ( @@ -11,7 +10,6 @@ INSERT INTO {{ .Ident "secret_encrypted_value" }} ( {{ .Arg .Row.Name }}, {{ .Arg .Row.Version }}, {{ .Arg .Row.EncryptedData }}, - {{ .Arg .Row.DataKeyID }}, {{ .Arg .Row.Created }}, {{ .Arg .Row.Updated }} ); diff --git a/pkg/storage/secret/encryption/data/encrypted_value_list_all.sql b/pkg/storage/secret/encryption/data/encrypted_value_list_all.sql index 2a0446a3a9d..d318517346d 100644 --- a/pkg/storage/secret/encryption/data/encrypted_value_list_all.sql +++ b/pkg/storage/secret/encryption/data/encrypted_value_list_all.sql @@ -3,7 +3,6 @@ SELECT {{ .Ident "name" }}, {{ .Ident "version" }}, {{ .Ident "encrypted_data" }}, - {{ .Ident "data_key_id" }}, {{ .Ident "created" }}, {{ .Ident "updated" }} FROM diff --git a/pkg/storage/secret/encryption/data/encrypted_value_read.sql b/pkg/storage/secret/encryption/data/encrypted_value_read.sql index 3ff20a641f0..6a672554efb 100644 --- a/pkg/storage/secret/encryption/data/encrypted_value_read.sql +++ b/pkg/storage/secret/encryption/data/encrypted_value_read.sql @@ -3,7 +3,6 @@ SELECT {{ .Ident "name" }}, {{ .Ident "version" }}, {{ .Ident "encrypted_data" }}, - {{ .Ident "data_key_id" }}, {{ .Ident "created" }}, {{ .Ident "updated" }} FROM diff --git a/pkg/storage/secret/encryption/data/encrypted_value_update.sql b/pkg/storage/secret/encryption/data/encrypted_value_update.sql index c939c092c1c..08e985297a1 100644 --- a/pkg/storage/secret/encryption/data/encrypted_value_update.sql +++ b/pkg/storage/secret/encryption/data/encrypted_value_update.sql @@ -2,7 +2,6 @@ UPDATE {{ .Ident "secret_encrypted_value" }} SET {{ .Ident "encrypted_data" }} = {{ .Arg .EncryptedData }}, - {{ .Ident "data_key_id" }} = {{ .Arg .DataKeyID }}, {{ .Ident "updated" }} = {{ .Arg .Updated }} WHERE {{ .Ident "namespace" }} = {{ .Arg .Namespace }} AND diff --git a/pkg/storage/secret/encryption/encrypted_value_model.go b/pkg/storage/secret/encryption/encrypted_value_model.go index 3c4ba7be905..eb66247c021 100644 --- a/pkg/storage/secret/encryption/encrypted_value_model.go +++ b/pkg/storage/secret/encryption/encrypted_value_model.go @@ -6,7 +6,6 @@ type EncryptedValue struct { Namespace string Name string Version int64 - DataKeyID string EncryptedData []byte Created int64 Updated int64 diff --git a/pkg/storage/secret/encryption/encrypted_value_store.go b/pkg/storage/secret/encryption/encrypted_value_store.go index af48a651868..3a10d01b957 100644 --- a/pkg/storage/secret/encryption/encrypted_value_store.go +++ b/pkg/storage/secret/encryption/encrypted_value_store.go @@ -1,9 +1,7 @@ package encryption import ( - "bytes" "context" - "encoding/base64" "errors" "fmt" "time" @@ -12,7 +10,6 @@ import ( "go.opentelemetry.io/otel/trace" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" - "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" "github.com/grafana/grafana/pkg/storage/unified/sql" "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" ) @@ -40,9 +37,9 @@ type encryptedValStorage struct { tracer trace.Tracer } -func (s *encryptedValStorage) Create(ctx context.Context, namespace xkube.Namespace, name string, version int64, encryptedData contracts.EncryptedPayload) (ev *contracts.EncryptedValue, err error) { +func (s *encryptedValStorage) Create(ctx context.Context, namespace, name string, version int64, encryptedData []byte) (ev *contracts.EncryptedValue, err error) { ctx, span := s.tracer.Start(ctx, "EncryptedValueStorage.Create", trace.WithAttributes( - attribute.String("namespace", namespace.String()), + attribute.String("namespace", namespace), )) defer span.End() @@ -59,11 +56,10 @@ func (s *encryptedValStorage) Create(ctx context.Context, namespace xkube.Namesp createdTime := time.Now().Unix() encryptedValue := &EncryptedValue{ - Namespace: namespace.String(), + Namespace: namespace, Name: name, Version: version, - EncryptedData: encryptedData.EncryptedData, - DataKeyID: encryptedData.DataKeyID, + EncryptedData: encryptedData, Created: createdTime, Updated: createdTime, } @@ -92,21 +88,18 @@ func (s *encryptedValStorage) Create(ctx context.Context, namespace xkube.Namesp } return &contracts.EncryptedValue{ - Namespace: encryptedValue.Namespace, - Name: encryptedValue.Name, - Version: encryptedValue.Version, - EncryptedPayload: contracts.EncryptedPayload{ - DataKeyID: encryptedValue.DataKeyID, - EncryptedData: encryptedValue.EncryptedData, - }, - Created: encryptedValue.Created, - Updated: encryptedValue.Updated, + Namespace: encryptedValue.Namespace, + Name: encryptedValue.Name, + Version: encryptedValue.Version, + EncryptedData: encryptedValue.EncryptedData, + Created: encryptedValue.Created, + Updated: encryptedValue.Updated, }, nil } -func (s *encryptedValStorage) Update(ctx context.Context, namespace xkube.Namespace, name string, version int64, encryptedData contracts.EncryptedPayload) error { +func (s *encryptedValStorage) Update(ctx context.Context, namespace, name string, version int64, encryptedData []byte) error { ctx, span := s.tracer.Start(ctx, "EncryptedValueStorage.Update", trace.WithAttributes( - attribute.String("namespace", namespace.String()), + attribute.String("namespace", namespace), attribute.String("name", name), attribute.Int64("version", version), )) @@ -114,11 +107,10 @@ func (s *encryptedValStorage) Update(ctx context.Context, namespace xkube.Namesp req := updateEncryptedValue{ SQLTemplate: sqltemplate.New(s.dialect), - Namespace: namespace.String(), + Namespace: namespace, Name: name, Version: version, - EncryptedData: encryptedData.EncryptedData, - DataKeyID: encryptedData.DataKeyID, + EncryptedData: encryptedData, Updated: time.Now().Unix(), } @@ -141,9 +133,9 @@ func (s *encryptedValStorage) Update(ctx context.Context, namespace xkube.Namesp return nil } -func (s *encryptedValStorage) Get(ctx context.Context, namespace xkube.Namespace, name string, version int64) (*contracts.EncryptedValue, error) { +func (s *encryptedValStorage) Get(ctx context.Context, namespace, name string, version int64) (*contracts.EncryptedValue, error) { ctx, span := s.tracer.Start(ctx, "EncryptedValueStorage.Get", trace.WithAttributes( - attribute.String("namespace", namespace.String()), + attribute.String("namespace", namespace), attribute.String("name", name), attribute.Int64("version", version), )) @@ -151,7 +143,7 @@ func (s *encryptedValStorage) Get(ctx context.Context, namespace xkube.Namespace req := &readEncryptedValue{ SQLTemplate: sqltemplate.New(s.dialect), - Namespace: namespace.String(), + Namespace: namespace, Name: name, Version: version, } @@ -171,7 +163,7 @@ func (s *encryptedValStorage) Get(ctx context.Context, namespace xkube.Namespace } var encryptedValue EncryptedValue - err = rows.Scan(&encryptedValue.Namespace, &encryptedValue.Name, &encryptedValue.Version, &encryptedValue.EncryptedData, &encryptedValue.DataKeyID, &encryptedValue.Created, &encryptedValue.Updated) + err = rows.Scan(&encryptedValue.Namespace, &encryptedValue.Name, &encryptedValue.Version, &encryptedValue.EncryptedData, &encryptedValue.Created, &encryptedValue.Updated) if err != nil { return nil, fmt.Errorf("failed to scan encrypted value row: %w", err) } @@ -180,21 +172,18 @@ func (s *encryptedValStorage) Get(ctx context.Context, namespace xkube.Namespace } return &contracts.EncryptedValue{ - Namespace: encryptedValue.Namespace, - Name: encryptedValue.Name, - Version: encryptedValue.Version, - EncryptedPayload: contracts.EncryptedPayload{ - DataKeyID: encryptedValue.DataKeyID, - EncryptedData: encryptedValue.EncryptedData, - }, - Created: encryptedValue.Created, - Updated: encryptedValue.Updated, + Namespace: encryptedValue.Namespace, + Name: encryptedValue.Name, + Version: encryptedValue.Version, + EncryptedData: encryptedValue.EncryptedData, + Created: encryptedValue.Created, + Updated: encryptedValue.Updated, }, nil } -func (s *encryptedValStorage) Delete(ctx context.Context, namespace xkube.Namespace, name string, version int64) error { +func (s *encryptedValStorage) Delete(ctx context.Context, namespace, name string, version int64) error { ctx, span := s.tracer.Start(ctx, "EncryptedValueStorage.Delete", trace.WithAttributes( - attribute.String("namespace", namespace.String()), + attribute.String("namespace", namespace), attribute.String("name", name), attribute.Int64("version", version), )) @@ -202,7 +191,7 @@ func (s *encryptedValStorage) Delete(ctx context.Context, namespace xkube.Namesp req := deleteEncryptedValue{ SQLTemplate: sqltemplate.New(s.dialect), - Namespace: namespace.String(), + Namespace: namespace, Name: name, Version: version, } @@ -275,7 +264,6 @@ func (s *globalEncryptedValStorage) ListAll(ctx context.Context, opts contracts. &row.Name, &row.Version, &row.EncryptedData, - &row.DataKeyID, &row.Created, &row.Updated, ) @@ -284,15 +272,12 @@ func (s *globalEncryptedValStorage) ListAll(ctx context.Context, opts contracts. } encryptedValues = append(encryptedValues, &contracts.EncryptedValue{ - Namespace: row.Namespace, - Name: row.Name, - Version: row.Version, - EncryptedPayload: contracts.EncryptedPayload{ - DataKeyID: row.DataKeyID, - EncryptedData: row.EncryptedData, - }, - Created: row.Created, - Updated: row.Updated, + Namespace: row.Namespace, + Name: row.Name, + Version: row.Version, + EncryptedData: row.EncryptedData, + Created: row.Created, + Updated: row.Updated, }) } if err := rows.Err(); err != nil { @@ -344,77 +329,3 @@ func (s *globalEncryptedValStorage) CountAll(ctx context.Context, untilTime *int return count, nil } - -type encryptedValMigrationExecutor struct { - db contracts.Database - dialect sqltemplate.Dialect - tracer trace.Tracer - encryptedValueStore contracts.EncryptedValueStorage - globalStore contracts.GlobalEncryptedValueStorage -} - -func ProvideEncryptedValueMigrationExecutor( - db contracts.Database, - tracer trace.Tracer, - encryptedValueStore contracts.EncryptedValueStorage, - globalStore contracts.GlobalEncryptedValueStorage, -) (contracts.EncryptedValueMigrationExecutor, error) { - return &encryptedValMigrationExecutor{ - db: db, - dialect: sqltemplate.DialectForDriver(db.DriverName()), - tracer: tracer, - encryptedValueStore: encryptedValueStore, - globalStore: globalStore, - }, nil -} - -func (s *encryptedValMigrationExecutor) Execute(ctx context.Context) (int, error) { - ctx, span := s.tracer.Start(ctx, "EncryptedValueMigrationExecutor.Execute") - defer span.End() - - // 1. Retrieve all encrypted values - encryptedValues, err := s.globalStore.ListAll(ctx, contracts.ListOpts{}, nil) - if err != nil { - return 0, fmt.Errorf("listing all encrypted values: %w", err) - } - - // This doesn't need to be done in a single transaction because there's no risk to successful rows if other rows fail - rowsAffected := 0 - for _, encryptedValue := range encryptedValues { - // 2. If the value already has the data key id broken out, skip it - if encryptedValue.DataKeyID != "" { - continue - } - - // 3. Split the data key id and the encrypted data out from the encoded payload - payload := encryptedValue.EncryptedData - const keyIdDelimiter = '#' - payload = payload[1:] - endOfKey := bytes.Index(payload, []byte{keyIdDelimiter}) - if endOfKey == -1 { - return rowsAffected, fmt.Errorf("could not find valid key id in encrypted payload with namespace %s and name %s and version %d", encryptedValue.Namespace, encryptedValue.Name, encryptedValue.Version) - } - b64Key := payload[:endOfKey] - encryptedData := payload[endOfKey+1:] - if len(encryptedData) == 0 { - return rowsAffected, fmt.Errorf("encrypted data is empty with namespace %s and name %s and version %d", encryptedValue.Namespace, encryptedValue.Name, encryptedValue.Version) - } - keyId := make([]byte, base64.RawStdEncoding.DecodedLen(len(b64Key))) - _, err := base64.RawStdEncoding.Decode(keyId, b64Key) - if err != nil { - return rowsAffected, fmt.Errorf("decoding key id with namespace %s and name %s and version %d: %w", encryptedValue.Namespace, encryptedValue.Name, encryptedValue.Version, err) - } - - // 4. Update the encrypted value with the data key id and the encrypted data - err = s.encryptedValueStore.Update(ctx, xkube.Namespace(encryptedValue.Namespace), encryptedValue.Name, encryptedValue.Version, contracts.EncryptedPayload{ - DataKeyID: string(keyId), - EncryptedData: encryptedData, - }) - if err != nil { - return rowsAffected, fmt.Errorf("updating encrypted value with namespace %s and name %s and version %d: %w", encryptedValue.Namespace, encryptedValue.Name, encryptedValue.Version, err) - } - rowsAffected++ - } - - return rowsAffected, nil -} diff --git a/pkg/storage/secret/encryption/encrypted_value_store_test.go b/pkg/storage/secret/encryption/encrypted_value_store_test.go index 2f84e1f8575..620a034f22c 100644 --- a/pkg/storage/secret/encryption/encrypted_value_store_test.go +++ b/pkg/storage/secret/encryption/encrypted_value_store_test.go @@ -2,25 +2,15 @@ package encryption_test import ( "bytes" - "context" - "encoding/base64" "errors" - "fmt" "slices" "testing" - "text/template" "time" - "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" - "github.com/grafana/grafana/pkg/registry/apis/secret/encryption/cipher" - cipherService "github.com/grafana/grafana/pkg/registry/apis/secret/encryption/cipher/service" "github.com/grafana/grafana/pkg/registry/apis/secret/testutils" - "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" "github.com/grafana/grafana/pkg/storage/secret/encryption" - "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" "github.com/stretchr/testify/require" - "go.opentelemetry.io/otel/trace/noop" "pgregory.net/rapid" ) @@ -31,10 +21,7 @@ func TestEncryptedValueStoreImpl(t *testing.T) { t.Parallel() sut := testutils.Setup(t) - createdEV, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace", "test-name", 1, contracts.EncryptedPayload{ - DataKeyID: "test-data-key-id", - EncryptedData: []byte("test-data"), - }) + createdEV, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace", "test-name", 1, []byte("test-data")) require.NoError(t, err) require.NotEmpty(t, createdEV.Namespace) require.NotEmpty(t, createdEV.Name) @@ -49,13 +36,10 @@ func TestEncryptedValueStoreImpl(t *testing.T) { t.Parallel() sut := testutils.Setup(t) - createdEV, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace", "test-name", 1, contracts.EncryptedPayload{ - DataKeyID: "test-data-key-id", - EncryptedData: []byte("test-data"), - }) + createdEV, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace", "test-name", 1, []byte("test-data")) require.NoError(t, err) - obtainedEV, err := sut.EncryptedValueStorage.Get(t.Context(), xkube.Namespace(createdEV.Namespace), createdEV.Name, createdEV.Version) + obtainedEV, err := sut.EncryptedValueStorage.Get(t.Context(), createdEV.Namespace, createdEV.Name, createdEV.Version) require.NoError(t, err) require.Equal(t, createdEV.Namespace, obtainedEV.Namespace) @@ -63,7 +47,6 @@ func TestEncryptedValueStoreImpl(t *testing.T) { require.Equal(t, createdEV.Created, obtainedEV.Created) require.Equal(t, createdEV.Updated, obtainedEV.Updated) require.Equal(t, createdEV.EncryptedData, obtainedEV.EncryptedData) - require.Equal(t, createdEV.DataKeyID, obtainedEV.DataKeyID) require.Equal(t, createdEV.Namespace, obtainedEV.Namespace) }) @@ -71,10 +54,7 @@ func TestEncryptedValueStoreImpl(t *testing.T) { t.Parallel() sut := testutils.Setup(t) - createdEV, err := sut.EncryptedValueStorage.Create(t.Context(), "ns1", "test-name", 1, contracts.EncryptedPayload{ - DataKeyID: "test-data-key-id", - EncryptedData: []byte("test-data"), - }) + createdEV, err := sut.EncryptedValueStorage.Create(t.Context(), "ns1", "test-name", 1, []byte("test-data")) require.NoError(t, err) obtainedEV, err := sut.EncryptedValueStorage.Get(t.Context(), "ns2", createdEV.Name, createdEV.Version) @@ -98,23 +78,16 @@ func TestEncryptedValueStoreImpl(t *testing.T) { t.Parallel() sut := testutils.Setup(t) - createdEV, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace", "test-name", 1, contracts.EncryptedPayload{ - DataKeyID: "test-data-key-id", - EncryptedData: []byte("test-data"), - }) + createdEV, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace", "test-name", 1, []byte("test-data")) require.NoError(t, err) - err = sut.EncryptedValueStorage.Update(t.Context(), xkube.Namespace(createdEV.Namespace), createdEV.Name, createdEV.Version, contracts.EncryptedPayload{ - DataKeyID: "test-data-key-id-updated", - EncryptedData: []byte("test-data-updated"), - }) + err = sut.EncryptedValueStorage.Update(t.Context(), createdEV.Namespace, createdEV.Name, createdEV.Version, []byte("test-data-updated")) require.NoError(t, err) - updatedEV, err := sut.EncryptedValueStorage.Get(t.Context(), xkube.Namespace(createdEV.Namespace), createdEV.Name, createdEV.Version) + updatedEV, err := sut.EncryptedValueStorage.Get(t.Context(), createdEV.Namespace, createdEV.Name, createdEV.Version) require.NoError(t, err) require.Equal(t, []byte("test-data-updated"), updatedEV.EncryptedData) - require.Equal(t, "test-data-key-id-updated", updatedEV.DataKeyID) require.Equal(t, createdEV.Created, updatedEV.Created) require.Equal(t, createdEV.Namespace, updatedEV.Namespace) }) @@ -123,10 +96,7 @@ func TestEncryptedValueStoreImpl(t *testing.T) { t.Parallel() sut := testutils.Setup(t) - err := sut.EncryptedValueStorage.Update(t.Context(), "test-namespace", "test-uid", 1, contracts.EncryptedPayload{ - DataKeyID: "test-data-key-id", - EncryptedData: []byte("test-data"), - }) + err := sut.EncryptedValueStorage.Update(t.Context(), "test-namespace", "test-uid", 1, []byte("test-data")) require.Error(t, err) }) @@ -134,19 +104,16 @@ func TestEncryptedValueStoreImpl(t *testing.T) { t.Parallel() sut := testutils.Setup(t) - createdEV, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace", "test-name", 1, contracts.EncryptedPayload{ - DataKeyID: "test-data-key-id", - EncryptedData: []byte("ttttest-data"), - }) + createdEV, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace", "test-name", 1, []byte("ttttest-data")) require.NoError(t, err) - _, err = sut.EncryptedValueStorage.Get(t.Context(), xkube.Namespace(createdEV.Namespace), createdEV.Name, createdEV.Version) + _, err = sut.EncryptedValueStorage.Get(t.Context(), createdEV.Namespace, createdEV.Name, createdEV.Version) require.NoError(t, err) - err = sut.EncryptedValueStorage.Delete(t.Context(), xkube.Namespace(createdEV.Namespace), createdEV.Name, createdEV.Version) + err = sut.EncryptedValueStorage.Delete(t.Context(), createdEV.Namespace, createdEV.Name, createdEV.Version) require.NoError(t, err) - obtainedEV, err := sut.EncryptedValueStorage.Get(t.Context(), xkube.Namespace(createdEV.Namespace), createdEV.Name, createdEV.Version) + obtainedEV, err := sut.EncryptedValueStorage.Get(t.Context(), createdEV.Namespace, createdEV.Name, createdEV.Version) require.Error(t, err) require.Nil(t, obtainedEV) }) @@ -163,16 +130,10 @@ func TestEncryptedValueStoreImpl(t *testing.T) { t.Parallel() sut := testutils.Setup(t) - createdEvA, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace-a", "test-name", 1, contracts.EncryptedPayload{ - DataKeyID: "test-data-key-id", - EncryptedData: []byte("test-data"), - }) + createdEvA, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace-a", "test-name", 1, []byte("test-data")) require.NoError(t, err) - createdEvB, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace-b", "test-name", 1, contracts.EncryptedPayload{ - DataKeyID: "test-data-key-id", - EncryptedData: []byte("test-data"), - }) + createdEvB, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace-b", "test-name", 1, []byte("test-data")) require.NoError(t, err) // List all encrypted values, without pagination @@ -219,16 +180,10 @@ func TestEncryptedValueStoreImpl(t *testing.T) { t.Parallel() sut := testutils.Setup(t) - _, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace-a", "test-name", 1, contracts.EncryptedPayload{ - DataKeyID: "test-data-key-id", - EncryptedData: []byte("test-data"), - }) + _, err := sut.EncryptedValueStorage.Create(t.Context(), "test-namespace-a", "test-name", 1, []byte("test-data")) require.NoError(t, err) - _, err = sut.EncryptedValueStorage.Create(t.Context(), "test-namespace-b", "test-name", 1, contracts.EncryptedPayload{ - DataKeyID: "test-data-key-id", - EncryptedData: []byte("test-data"), - }) + _, err = sut.EncryptedValueStorage.Create(t.Context(), "test-namespace-b", "test-name", 1, []byte("test-data")) require.NoError(t, err) count, err := sut.GlobalEncryptedValueStorage.CountAll(t.Context(), nil) @@ -243,281 +198,6 @@ func TestEncryptedValueStoreImpl(t *testing.T) { }) } -func TestEncryptedValueMigration(t *testing.T) { - t.Parallel() - - t.Run("golden path - successful migration of legacy format", func(t *testing.T) { - t.Parallel() - - sut := testutils.Setup(t) - tracer := noop.NewTracerProvider().Tracer("test") - usageStats := &usagestats.UsageStatsMock{T: t} - enc, err := cipherService.ProvideAESGCMCipherService(tracer, usageStats) - require.NoError(t, err) - - testCases := []struct { - namespace string - name string - version int64 - plaintext string - dataKeyId string - }{ - { - namespace: "test-namespace-1", - name: "test-name-1", - version: 1, - plaintext: "test-plaintext-1", - dataKeyId: "test-data-key-id-1", - }, - { - namespace: "test-namespace-1", - name: "test-name-2", - version: 1, - plaintext: "test-plaintext-2", - dataKeyId: "test-data-key-id-1", - }, - { - namespace: "test-namespace-2", - name: "test-name-3", - version: 1, - plaintext: "test-plaintext-3", - dataKeyId: "test-data-key-id-2", - }, - } - - // Seed with data in the legacy format - for _, tc := range testCases { - err := createLegacyEncryptedData(t, sut, enc, tc.namespace, tc.name, tc.version, tc.plaintext, tc.dataKeyId) - require.NoError(t, err) - } - - // Run the migration and blindy trust it - rowsAffected, err := sut.EncryptedValueMigrationExecutor.Execute(t.Context()) - require.NoError(t, err) - require.Equal(t, len(testCases), rowsAffected) - - // Now validate that the data is in the new format - encryptedValues, err := sut.GlobalEncryptedValueStorage.ListAll(t.Context(), contracts.ListOpts{}, nil) - require.NoError(t, err) - require.Len(t, encryptedValues, 3) - - for _, tc := range testCases { - ev, err := sut.EncryptedValueStorage.Get(t.Context(), xkube.Namespace(tc.namespace), tc.name, tc.version) - require.NoError(t, err) - - // Decrypt the encrypted data and check for equality - decrypted, err := enc.Decrypt(t.Context(), ev.EncryptedData, tc.dataKeyId) - require.NoError(t, err) - require.Equal(t, tc.dataKeyId, ev.DataKeyID) - require.Equal(t, tc.plaintext, string(decrypted)) - } - }) - - t.Run("error conditions - handles corrupt data gracefully", func(t *testing.T) { - t.Parallel() - - tracer := noop.NewTracerProvider().Tracer("test") - sut := testutils.Setup(t) - - t.Run("global store list error", func(t *testing.T) { - mockGlobalStore := &mockGlobalEncryptedValueStorage{ - listAllError: errors.New("database connection failed"), - } - - migrationExecutor, err := encryption.ProvideEncryptedValueMigrationExecutor( - sut.Database, - tracer, - sut.EncryptedValueStorage, - mockGlobalStore, - ) - require.NoError(t, err) - - rowsAffected, err := migrationExecutor.Execute(t.Context()) - require.Error(t, err) - require.Contains(t, err.Error(), "listing all encrypted values") - require.Equal(t, 0, rowsAffected) - }) - - t.Run("corrupt data - missing key delimiter", func(t *testing.T) { - mockGlobalStore := &mockGlobalEncryptedValueStorage{ - encryptedValues: []*contracts.EncryptedValue{ - { - Namespace: "test-ns", - Name: "test-name", - Version: 1, - EncryptedPayload: contracts.EncryptedPayload{ - EncryptedData: []byte("corrupt-data-without-delimiter"), - DataKeyID: "", // Empty to trigger migration - }, - }, - }, - } - - migrationExecutor, err := encryption.ProvideEncryptedValueMigrationExecutor( - sut.Database, - tracer, - sut.EncryptedValueStorage, - mockGlobalStore, - ) - require.NoError(t, err) - - rowsAffected, err := migrationExecutor.Execute(t.Context()) - require.Error(t, err) - require.Contains(t, err.Error(), "could not find valid key id in encrypted payload") - require.Equal(t, 0, rowsAffected) - }) - - t.Run("corrupt data - empty encrypted data", func(t *testing.T) { - mockGlobalStore := &mockGlobalEncryptedValueStorage{ - encryptedValues: []*contracts.EncryptedValue{ - { - Namespace: "test-ns", - Name: "test-name", - Version: 1, - EncryptedPayload: contracts.EncryptedPayload{ - EncryptedData: []byte("#dGVzdA#"), // Valid key but no encrypted data after delimiter - DataKeyID: "", // Empty to trigger migration - }, - }, - }, - } - - migrationExecutor, err := encryption.ProvideEncryptedValueMigrationExecutor( - sut.Database, - tracer, - sut.EncryptedValueStorage, - mockGlobalStore, - ) - require.NoError(t, err) - - rowsAffected, err := migrationExecutor.Execute(t.Context()) - require.Error(t, err) - require.Contains(t, err.Error(), "encrypted data is empty") - require.Equal(t, 0, rowsAffected) - }) - - t.Run("corrupt data - invalid base64 key", func(t *testing.T) { - mockGlobalStore := &mockGlobalEncryptedValueStorage{ - encryptedValues: []*contracts.EncryptedValue{ - { - Namespace: "test-ns", - Name: "test-name", - Version: 1, - EncryptedPayload: contracts.EncryptedPayload{ - EncryptedData: []byte("#invalid-base64!@#$%^&*()#somedata"), - DataKeyID: "", // Empty to trigger migration - }, - }, - }, - } - - migrationExecutor, err := encryption.ProvideEncryptedValueMigrationExecutor( - sut.Database, - tracer, - sut.EncryptedValueStorage, - mockGlobalStore, - ) - require.NoError(t, err) - - rowsAffected, err := migrationExecutor.Execute(t.Context()) - require.Error(t, err) - require.Contains(t, err.Error(), "decoding key id") - require.Equal(t, 0, rowsAffected) - }) - - t.Run("update failure", func(t *testing.T) { - mockGlobalStore := &mockGlobalEncryptedValueStorage{ - encryptedValues: []*contracts.EncryptedValue{ - { - Namespace: "nonexistent-ns", - Name: "nonexistent-name", - Version: 999, - EncryptedPayload: contracts.EncryptedPayload{ - EncryptedData: []byte("#dGVzdA#someencrypteddata"), - DataKeyID: "", // Empty to trigger migration - }, - }, - }, - } - - migrationExecutor, err := encryption.ProvideEncryptedValueMigrationExecutor( - sut.Database, - tracer, - sut.EncryptedValueStorage, - mockGlobalStore, - ) - require.NoError(t, err) - - rowsAffected, err := migrationExecutor.Execute(t.Context()) - require.Error(t, err) - require.Contains(t, err.Error(), "updating encrypted value") - require.Equal(t, 0, rowsAffected) - }) - }) -} - -// Helper function that bypasses interfaces and creates data in the legacy format directly in the database. -// The format is "#{encoded_key_id}#{encrypted_data}". -func createLegacyEncryptedData(t *testing.T, sut testutils.Sut, enc cipher.Cipher, namespace, name string, version int64, plaintext string, dataKeyId string) error { - t.Helper() - - encryptedData, err := enc.Encrypt(t.Context(), []byte(plaintext), dataKeyId) - require.NoError(t, err) - - // Encode using the legacy format - const keyIdDelimiter = '#' - prefix := make([]byte, base64.RawStdEncoding.EncodedLen(len(dataKeyId))+2) - base64.RawStdEncoding.Encode(prefix[1:], []byte(dataKeyId)) - prefix[0] = keyIdDelimiter - prefix[len(prefix)-1] = keyIdDelimiter - - blob := make([]byte, len(prefix)+len(encryptedData)) - copy(blob, prefix) - copy(blob[len(prefix):], encryptedData) - - createdTime := time.Now().Unix() - - encryptedValue := &encryption.EncryptedValue{ - Namespace: namespace, - Name: name, - Version: version, - EncryptedData: blob, - DataKeyID: "", - Created: createdTime, - Updated: createdTime, - } - - req := struct { - sqltemplate.SQLTemplate - Row *encryption.EncryptedValue - }{ - SQLTemplate: sqltemplate.New(sqltemplate.DialectForDriver(sut.Database.DriverName())), - Row: encryptedValue, - } - tmpl, err := template.ParseFiles("data/encrypted_value_create.sql") - if err != nil { - return fmt.Errorf("parsing template: %w", err) - } - - query, err := sqltemplate.Execute(tmpl, req) - if err != nil { - return fmt.Errorf("executing template: %w", err) - } - - res, err := sut.Database.ExecContext(t.Context(), query, req.GetArgs()...) - if err != nil { - return fmt.Errorf("inserting row: %w", err) - } - - if rowsAffected, err := res.RowsAffected(); err != nil { - return fmt.Errorf("getting rows affected: %w", err) - } else if rowsAffected != 1 { - return fmt.Errorf("expected 1 row affected, got %d", rowsAffected) - } - - return nil -} - func TestStateMachine(t *testing.T) { t.Parallel() @@ -532,14 +212,10 @@ func TestStateMachine(t *testing.T) { ns := namespaceGen.Draw(t, "ns") name := nameGen.Draw(t, "name") version := versionGen.Draw(t, "version") - dataKeyId := rapid.String().Draw(t, "dataKeyId") plaintext := rapid.String().Draw(t, "plaintext") - _, modelErr := m.create(ns, name, version, []byte(plaintext), dataKeyId) - _, err := sut.EncryptedValueStorage.Create(t.Context(), xkube.Namespace(ns), name, version, contracts.EncryptedPayload{ - DataKeyID: dataKeyId, - EncryptedData: []byte(plaintext), - }) + _, modelErr := m.create(ns, name, version, []byte(plaintext)) + _, err := sut.EncryptedValueStorage.Create(t.Context(), ns, name, version, []byte(plaintext)) if modelErr != nil || err != nil { require.ErrorIs(t, err, modelErr) return @@ -549,14 +225,10 @@ func TestStateMachine(t *testing.T) { ns := namespaceGen.Draw(t, "ns") name := nameGen.Draw(t, "name") version := versionGen.Draw(t, "version") - dataKeyId := rapid.String().Draw(t, "dataKeyId") plaintext := rapid.String().Draw(t, "plaintext") - modelErr := m.update(ns, name, version, []byte(plaintext), dataKeyId) - err := sut.EncryptedValueStorage.Update(t.Context(), xkube.Namespace(ns), name, version, contracts.EncryptedPayload{ - DataKeyID: dataKeyId, - EncryptedData: []byte(plaintext), - }) + modelErr := m.update(ns, name, version, []byte(plaintext)) + err := sut.EncryptedValueStorage.Update(t.Context(), ns, name, version, []byte(plaintext)) if modelErr != nil || err != nil { require.ErrorIs(t, err, modelErr) return @@ -568,7 +240,7 @@ func TestStateMachine(t *testing.T) { version := versionGen.Draw(t, "version") modelValue, modelErr := m.get(ns, name, version) - value, err := sut.EncryptedValueStorage.Get(t.Context(), xkube.Namespace(ns), name, version) + value, err := sut.EncryptedValueStorage.Get(t.Context(), ns, name, version) if modelErr != nil || err != nil { require.ErrorIs(t, err, modelErr) return @@ -586,7 +258,7 @@ func TestStateMachine(t *testing.T) { version := versionGen.Draw(t, "version") modelErr := m.delete(ns, name, version) - err := sut.EncryptedValueStorage.Delete(t.Context(), xkube.Namespace(ns), name, version) + err := sut.EncryptedValueStorage.Delete(t.Context(), ns, name, version) if modelErr != nil || err != nil { require.ErrorIs(t, err, modelErr) return @@ -618,19 +290,18 @@ type entry struct { name string version int64 encryptedData []byte - dataKeyId string } func newModel() *model { return &model{} } -func (m *model) create(namespace, name string, version int64, encryptedData []byte, dataKeyId string) (*contracts.EncryptedValue, error) { +func (m *model) create(namespace, name string, version int64, encryptedData []byte) (*contracts.EncryptedValue, error) { v, err := m.get(namespace, name, version) if err != nil && !errors.Is(err, encryption.ErrEncryptedValueNotFound) { return nil, err } - // The entry being created already exists + // The entry being creted already exists if v != nil { return nil, encryption.ErrEncryptedValueAlreadyExists } @@ -640,25 +311,20 @@ func (m *model) create(namespace, name string, version int64, encryptedData []by name: name, version: version, encryptedData: encryptedData, - dataKeyId: dataKeyId, }) return &contracts.EncryptedValue{ - Namespace: namespace, - Name: name, - Version: version, - EncryptedPayload: contracts.EncryptedPayload{ - DataKeyID: dataKeyId, - EncryptedData: encryptedData, - }, - Created: 1, - Updated: 1, + Namespace: namespace, + Name: name, + Version: version, + EncryptedData: encryptedData, + Created: 1, + Updated: 1, }, nil } -func (m *model) update(namespace, name string, version int64, encryptedData []byte, dataKeyId string) error { +func (m *model) update(namespace, name string, version int64, encryptedData []byte) error { for _, v := range m.entries { if v.namespace == namespace && v.name == name && v.version == version { v.encryptedData = encryptedData - v.dataKeyId = dataKeyId return nil } } @@ -670,15 +336,12 @@ func (m *model) get(namespace, name string, version int64) (*contracts.Encrypted for _, v := range m.entries { if v.namespace == namespace && v.name == name && v.version == version { return &contracts.EncryptedValue{ - Namespace: namespace, - Name: name, - Version: version, - EncryptedPayload: contracts.EncryptedPayload{ - DataKeyID: v.dataKeyId, - EncryptedData: v.encryptedData, - }, - Created: 1, - Updated: 1, + Namespace: namespace, + Name: name, + Version: version, + EncryptedData: v.encryptedData, + Created: 1, + Updated: 1, }, nil } } @@ -691,25 +354,3 @@ func (m *model) delete(namespace, name string, version int64) error { }) return nil } - -// mockGlobalEncryptedValueStorage is a mock implementation of contracts.GlobalEncryptedValueStorage -// used for testing error conditions in the migration executor -type mockGlobalEncryptedValueStorage struct { - encryptedValues []*contracts.EncryptedValue - listAllError error - countAllError error -} - -func (m *mockGlobalEncryptedValueStorage) ListAll(ctx context.Context, opts contracts.ListOpts, untilTime *int64) ([]*contracts.EncryptedValue, error) { - if m.listAllError != nil { - return nil, m.listAllError - } - return m.encryptedValues, nil -} - -func (m *mockGlobalEncryptedValueStorage) CountAll(ctx context.Context, untilTime *int64) (int64, error) { - if m.countAllError != nil { - return 0, m.countAllError - } - return int64(len(m.encryptedValues)), nil -} diff --git a/pkg/storage/secret/encryption/query.go b/pkg/storage/secret/encryption/query.go index 21e6081a7c7..47ea83ce0cd 100644 --- a/pkg/storage/secret/encryption/query.go +++ b/pkg/storage/secret/encryption/query.go @@ -74,7 +74,6 @@ type updateEncryptedValue struct { Name string Version int64 EncryptedData []byte - DataKeyID string Updated int64 } diff --git a/pkg/storage/secret/encryption/query_test.go b/pkg/storage/secret/encryption/query_test.go index 4cbb830f0af..a93ff0b77e6 100644 --- a/pkg/storage/secret/encryption/query_test.go +++ b/pkg/storage/secret/encryption/query_test.go @@ -24,7 +24,6 @@ func TestEncryptedValueQueries(t *testing.T) { Name: "n1", Version: 1, EncryptedData: []byte("secret"), - DataKeyID: "test-data-key-id", Created: 1234, Updated: 5678, }, @@ -51,7 +50,6 @@ func TestEncryptedValueQueries(t *testing.T) { Name: "n1", Version: 1, EncryptedData: []byte("secret"), - DataKeyID: "test-data-key-id", Updated: 5679, }, }, diff --git a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_create-create.sql b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_create-create.sql index 952c04730a8..7a886b1c6ee 100755 --- a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_create-create.sql +++ b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_create-create.sql @@ -3,7 +3,6 @@ INSERT INTO `secret_encrypted_value` ( `name`, `version`, `encrypted_data`, - `data_key_id`, `created`, `updated` ) VALUES ( @@ -11,7 +10,6 @@ INSERT INTO `secret_encrypted_value` ( 'n1', 1, '[115 101 99 114 101 116]', - 'test-data-key-id', 1234, 5678 ); diff --git a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_all.sql b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_all.sql index e41d58588d4..74b699d45f1 100755 --- a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_all.sql +++ b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_all.sql @@ -3,7 +3,6 @@ SELECT `name`, `version`, `encrypted_data`, - `data_key_id`, `created`, `updated` FROM diff --git a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_all_until_time.sql b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_all_until_time.sql index c51a40a9a5a..b34496aaf93 100755 --- a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_all_until_time.sql +++ b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_all_until_time.sql @@ -3,7 +3,6 @@ SELECT `name`, `version`, `encrypted_data`, - `data_key_id`, `created`, `updated` FROM diff --git a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_limit_10_offset_0.sql b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_limit_10_offset_0.sql index de82acacbbd..9c33fd6bdbf 100755 --- a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_limit_10_offset_0.sql +++ b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_limit_10_offset_0.sql @@ -3,7 +3,6 @@ SELECT `name`, `version`, `encrypted_data`, - `data_key_id`, `created`, `updated` FROM diff --git a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_limit_10_offset_2.sql b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_limit_10_offset_2.sql index f21f7122646..d7066395a78 100755 --- a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_limit_10_offset_2.sql +++ b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_list_all-list_limit_10_offset_2.sql @@ -3,7 +3,6 @@ SELECT `name`, `version`, `encrypted_data`, - `data_key_id`, `created`, `updated` FROM diff --git a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_read-read.sql b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_read-read.sql index bca020a26d3..bb9faddb8ff 100755 --- a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_read-read.sql +++ b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_read-read.sql @@ -3,7 +3,6 @@ SELECT `name`, `version`, `encrypted_data`, - `data_key_id`, `created`, `updated` FROM diff --git a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_update-update.sql b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_update-update.sql index b454cb5165a..cf3d6897a64 100755 --- a/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_update-update.sql +++ b/pkg/storage/secret/encryption/testdata/mysql--encrypted_value_update-update.sql @@ -2,7 +2,6 @@ UPDATE `secret_encrypted_value` SET `encrypted_data` = '[115 101 99 114 101 116]', - `data_key_id` = 'test-data-key-id', `updated` = 5679 WHERE `namespace` = 'ns' AND diff --git a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_create-create.sql b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_create-create.sql index 17576f07232..23d9a0f2331 100755 --- a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_create-create.sql +++ b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_create-create.sql @@ -3,7 +3,6 @@ INSERT INTO "secret_encrypted_value" ( "name", "version", "encrypted_data", - "data_key_id", "created", "updated" ) VALUES ( @@ -11,7 +10,6 @@ INSERT INTO "secret_encrypted_value" ( 'n1', 1, '[115 101 99 114 101 116]', - 'test-data-key-id', 1234, 5678 ); diff --git a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_all.sql b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_all.sql index ee8b9c0a399..74432ebbf69 100755 --- a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_all.sql +++ b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_all.sql @@ -3,7 +3,6 @@ SELECT "name", "version", "encrypted_data", - "data_key_id", "created", "updated" FROM diff --git a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_all_until_time.sql b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_all_until_time.sql index 661d7f9ea0a..1d7089f751e 100755 --- a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_all_until_time.sql +++ b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_all_until_time.sql @@ -3,7 +3,6 @@ SELECT "name", "version", "encrypted_data", - "data_key_id", "created", "updated" FROM diff --git a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_limit_10_offset_0.sql b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_limit_10_offset_0.sql index 06782d7e40d..6f2bbd0b90f 100755 --- a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_limit_10_offset_0.sql +++ b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_limit_10_offset_0.sql @@ -3,7 +3,6 @@ SELECT "name", "version", "encrypted_data", - "data_key_id", "created", "updated" FROM diff --git a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_limit_10_offset_2.sql b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_limit_10_offset_2.sql index a64d45afaec..b9f326c8bf0 100755 --- a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_limit_10_offset_2.sql +++ b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_list_all-list_limit_10_offset_2.sql @@ -3,7 +3,6 @@ SELECT "name", "version", "encrypted_data", - "data_key_id", "created", "updated" FROM diff --git a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_read-read.sql b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_read-read.sql index 437685ae4e7..4bbe1000ce6 100755 --- a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_read-read.sql +++ b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_read-read.sql @@ -3,7 +3,6 @@ SELECT "name", "version", "encrypted_data", - "data_key_id", "created", "updated" FROM diff --git a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_update-update.sql b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_update-update.sql index 0f710f7456d..b2b5e30ecac 100755 --- a/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_update-update.sql +++ b/pkg/storage/secret/encryption/testdata/postgres--encrypted_value_update-update.sql @@ -2,7 +2,6 @@ UPDATE "secret_encrypted_value" SET "encrypted_data" = '[115 101 99 114 101 116]', - "data_key_id" = 'test-data-key-id', "updated" = 5679 WHERE "namespace" = 'ns' AND diff --git a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_create-create.sql b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_create-create.sql index 17576f07232..23d9a0f2331 100755 --- a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_create-create.sql +++ b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_create-create.sql @@ -3,7 +3,6 @@ INSERT INTO "secret_encrypted_value" ( "name", "version", "encrypted_data", - "data_key_id", "created", "updated" ) VALUES ( @@ -11,7 +10,6 @@ INSERT INTO "secret_encrypted_value" ( 'n1', 1, '[115 101 99 114 101 116]', - 'test-data-key-id', 1234, 5678 ); diff --git a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_all.sql b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_all.sql index ee8b9c0a399..74432ebbf69 100755 --- a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_all.sql +++ b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_all.sql @@ -3,7 +3,6 @@ SELECT "name", "version", "encrypted_data", - "data_key_id", "created", "updated" FROM diff --git a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_all_until_time.sql b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_all_until_time.sql index 661d7f9ea0a..1d7089f751e 100755 --- a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_all_until_time.sql +++ b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_all_until_time.sql @@ -3,7 +3,6 @@ SELECT "name", "version", "encrypted_data", - "data_key_id", "created", "updated" FROM diff --git a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_limit_10_offset_0.sql b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_limit_10_offset_0.sql index 06782d7e40d..6f2bbd0b90f 100755 --- a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_limit_10_offset_0.sql +++ b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_limit_10_offset_0.sql @@ -3,7 +3,6 @@ SELECT "name", "version", "encrypted_data", - "data_key_id", "created", "updated" FROM diff --git a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_limit_10_offset_2.sql b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_limit_10_offset_2.sql index a64d45afaec..b9f326c8bf0 100755 --- a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_limit_10_offset_2.sql +++ b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_list_all-list_limit_10_offset_2.sql @@ -3,7 +3,6 @@ SELECT "name", "version", "encrypted_data", - "data_key_id", "created", "updated" FROM diff --git a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_read-read.sql b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_read-read.sql index 437685ae4e7..4bbe1000ce6 100755 --- a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_read-read.sql +++ b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_read-read.sql @@ -3,7 +3,6 @@ SELECT "name", "version", "encrypted_data", - "data_key_id", "created", "updated" FROM diff --git a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_update-update.sql b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_update-update.sql index 0f710f7456d..b2b5e30ecac 100755 --- a/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_update-update.sql +++ b/pkg/storage/secret/encryption/testdata/sqlite--encrypted_value_update-update.sql @@ -2,7 +2,6 @@ UPDATE "secret_encrypted_value" SET "encrypted_data" = '[115 101 99 114 101 116]', - "data_key_id" = 'test-data-key-id', "updated" = 5679 WHERE "namespace" = 'ns' AND diff --git a/pkg/storage/secret/metadata/decrypt_store.go b/pkg/storage/secret/metadata/decrypt_store.go index bd8ad2f73b9..d148e365925 100644 --- a/pkg/storage/secret/metadata/decrypt_store.go +++ b/pkg/storage/secret/metadata/decrypt_store.go @@ -134,7 +134,7 @@ func (s *decryptStorage) Decrypt(ctx context.Context, namespace xkube.Namespace, return "", fmt.Errorf("failed to get keeper for config: %v (%w)", err, contracts.ErrDecryptFailed) } - exposedValue, err := keeper.Expose(ctx, keeperConfig, namespace, name, sv.Status.Version) + exposedValue, err := keeper.Expose(ctx, keeperConfig, namespace.String(), name, sv.Status.Version) if err != nil { return "", fmt.Errorf("failed to expose secret: %v (%w)", err, contracts.ErrDecryptFailed) } diff --git a/pkg/storage/secret/migrator/migrator.go b/pkg/storage/secret/migrator/migrator.go index b08fb766e96..34b56a905df 100644 --- a/pkg/storage/secret/migrator/migrator.go +++ b/pkg/storage/secret/migrator/migrator.go @@ -200,15 +200,4 @@ func (*SecretDB) AddMigration(mg *migrator.Migrator) { mg.AddMigration("add lease_created index to "+TableNameSecureValue, migrator.NewAddIndexMigration(secureValueTable, &migrator.Index{ Cols: []string{"lease_created"}, })) - - mg.AddMigration("add data_key_id column to "+TableNameEncryptedValue, migrator.NewAddColumnMigration(encryptedValueTable, &migrator.Column{ - Name: "data_key_id", - Type: migrator.DB_NVarchar, - Length: 100, - Nullable: false, - Default: "''", - })) - mg.AddMigration("add data_key_id index to "+TableNameEncryptedValue, migrator.NewAddIndexMigration(encryptedValueTable, &migrator.Index{ - Cols: []string{"data_key_id"}, - })) } diff --git a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json index 3e2e3a1644b..bdfa6ce9490 100644 --- a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json @@ -3706,14 +3706,14 @@ "group": { "type": "string" }, + "kind": { + "type": "string" + }, "noop": { "description": "No action required (useful for sync)", "type": "integer", "format": "int64" }, - "resource": { - "type": "string" - }, "total": { "type": "integer", "format": "int64" diff --git a/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts b/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts index b43299a4d8a..41e4cac3c01 100644 --- a/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts +++ b/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts @@ -1010,9 +1010,9 @@ export type JobResourceSummary = { /** Report errors for this resource type This may not be an exhaustive list and recommend looking at the logs for more info */ errors?: string[]; group?: string; + kind?: string; /** No action required (useful for sync) */ noop?: number; - resource?: string; total?: number; update?: number; write?: number; diff --git a/public/app/features/provisioning/Job/JobSummary.tsx b/public/app/features/provisioning/Job/JobSummary.tsx index f91b28a261f..0f9ba17db08 100644 --- a/public/app/features/provisioning/Job/JobSummary.tsx +++ b/public/app/features/provisioning/Job/JobSummary.tsx @@ -11,7 +11,7 @@ const getSummaryColumns = () => [ { id: 'resource', header: 'Resource', - cell: ({ row: { original: item } }: SummaryCell) => item.resource, + cell: ({ row: { original: item } }: SummaryCell) => item.kind, }, { id: 'created', @@ -58,7 +58,7 @@ export function JobSummary({ summary }: Props) { item.resource || ''} + getRowId={(item) => item.kind || ''} pageSize={10} />