diff --git a/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go b/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go index fc24ac84869..dff7c3813e5 100644 --- a/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go +++ b/apps/provisioning/pkg/apis/provisioning/v0alpha1/types.go @@ -310,6 +310,9 @@ type RepositoryStatus struct { // Webhook Information (if applicable) Webhook *WebhookStatus `json:"webhook"` + + // Error information during repository deletion (if any) + DeleteError string `json:"deleteError,omitempty"` } // HealthFailureType represents different types of repository failures 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 e68b03458ac..74f7b7d165f 100644 --- a/apps/provisioning/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go +++ b/apps/provisioning/pkg/apis/provisioning/v0alpha1/zz_generated.openapi.go @@ -1587,6 +1587,13 @@ func schema_pkg_apis_provisioning_v0alpha1_RepositoryStatus(ref common.Reference Ref: ref("github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1.WebhookStatus"), }, }, + "deleteError": { + SchemaProps: spec.SchemaProps{ + Description: "Error information during repository deletion (if any)", + Type: []string{"string"}, + Format: "", + }, + }, }, Required: []string{"observedGeneration", "health", "sync", "webhook"}, }, diff --git a/apps/provisioning/pkg/generated/applyconfiguration/provisioning/v0alpha1/repositorystatus.go b/apps/provisioning/pkg/generated/applyconfiguration/provisioning/v0alpha1/repositorystatus.go index aebb18acafd..b1f57147335 100644 --- a/apps/provisioning/pkg/generated/applyconfiguration/provisioning/v0alpha1/repositorystatus.go +++ b/apps/provisioning/pkg/generated/applyconfiguration/provisioning/v0alpha1/repositorystatus.go @@ -12,6 +12,7 @@ type RepositoryStatusApplyConfiguration struct { Sync *SyncStatusApplyConfiguration `json:"sync,omitempty"` Stats []ResourceCountApplyConfiguration `json:"stats,omitempty"` Webhook *WebhookStatusApplyConfiguration `json:"webhook,omitempty"` + DeleteError *string `json:"deleteError,omitempty"` } // RepositoryStatusApplyConfiguration constructs a declarative configuration of the RepositoryStatus type for use with @@ -64,3 +65,11 @@ func (b *RepositoryStatusApplyConfiguration) WithWebhook(value *WebhookStatusApp b.Webhook = value return b } + +// WithDeleteError sets the DeleteError 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 DeleteError field is set to the value of the last call. +func (b *RepositoryStatusApplyConfiguration) WithDeleteError(value string) *RepositoryStatusApplyConfiguration { + b.DeleteError = &value + return b +} diff --git a/apps/provisioning/pkg/repository/factory.go b/apps/provisioning/pkg/repository/factory.go index 4220bc32adc..537c51fd5cc 100644 --- a/apps/provisioning/pkg/repository/factory.go +++ b/apps/provisioning/pkg/repository/factory.go @@ -18,7 +18,7 @@ type Extra interface { Mutate(ctx context.Context, obj runtime.Object) error } -//go:generate mockery --name=Factor --structname=MockFactory --inpackage --filename=factory_mock.go --with-expecter +//go:generate mockery --name=Factory --structname=MockFactory --inpackage --filename=factory_mock.go --with-expecter type Factory interface { Types() []provisioning.RepositoryType Build(ctx context.Context, r *provisioning.Repository) (Repository, error) diff --git a/apps/provisioning/pkg/repository/factory_mock.go b/apps/provisioning/pkg/repository/factory_mock.go new file mode 100644 index 00000000000..c6016d61bf1 --- /dev/null +++ b/apps/provisioning/pkg/repository/factory_mock.go @@ -0,0 +1,192 @@ +// Code generated by mockery v2.53.4. DO NOT EDIT. + +package repository + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" + runtime "k8s.io/apimachinery/pkg/runtime" + + v0alpha1 "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" +) + +// MockFactory is an autogenerated mock type for the Factory type +type MockFactory struct { + mock.Mock +} + +type MockFactory_Expecter struct { + mock *mock.Mock +} + +func (_m *MockFactory) EXPECT() *MockFactory_Expecter { + return &MockFactory_Expecter{mock: &_m.Mock} +} + +// Build provides a mock function with given fields: ctx, r +func (_m *MockFactory) Build(ctx context.Context, r *v0alpha1.Repository) (Repository, error) { + ret := _m.Called(ctx, r) + + if len(ret) == 0 { + panic("no return value specified for Build") + } + + var r0 Repository + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, *v0alpha1.Repository) (Repository, error)); ok { + return rf(ctx, r) + } + if rf, ok := ret.Get(0).(func(context.Context, *v0alpha1.Repository) Repository); ok { + r0 = rf(ctx, r) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(Repository) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, *v0alpha1.Repository) error); ok { + r1 = rf(ctx, r) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockFactory_Build_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Build' +type MockFactory_Build_Call struct { + *mock.Call +} + +// Build is a helper method to define mock.On call +// - ctx context.Context +// - r *v0alpha1.Repository +func (_e *MockFactory_Expecter) Build(ctx interface{}, r interface{}) *MockFactory_Build_Call { + return &MockFactory_Build_Call{Call: _e.mock.On("Build", ctx, r)} +} + +func (_c *MockFactory_Build_Call) Run(run func(ctx context.Context, r *v0alpha1.Repository)) *MockFactory_Build_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(*v0alpha1.Repository)) + }) + return _c +} + +func (_c *MockFactory_Build_Call) Return(_a0 Repository, _a1 error) *MockFactory_Build_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockFactory_Build_Call) RunAndReturn(run func(context.Context, *v0alpha1.Repository) (Repository, error)) *MockFactory_Build_Call { + _c.Call.Return(run) + return _c +} + +// Mutate provides a mock function with given fields: ctx, obj +func (_m *MockFactory) Mutate(ctx context.Context, obj runtime.Object) error { + ret := _m.Called(ctx, obj) + + if len(ret) == 0 { + panic("no return value specified for Mutate") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, runtime.Object) error); ok { + r0 = rf(ctx, obj) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MockFactory_Mutate_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Mutate' +type MockFactory_Mutate_Call struct { + *mock.Call +} + +// Mutate is a helper method to define mock.On call +// - ctx context.Context +// - obj runtime.Object +func (_e *MockFactory_Expecter) Mutate(ctx interface{}, obj interface{}) *MockFactory_Mutate_Call { + return &MockFactory_Mutate_Call{Call: _e.mock.On("Mutate", ctx, obj)} +} + +func (_c *MockFactory_Mutate_Call) Run(run func(ctx context.Context, obj runtime.Object)) *MockFactory_Mutate_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(runtime.Object)) + }) + return _c +} + +func (_c *MockFactory_Mutate_Call) Return(_a0 error) *MockFactory_Mutate_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockFactory_Mutate_Call) RunAndReturn(run func(context.Context, runtime.Object) error) *MockFactory_Mutate_Call { + _c.Call.Return(run) + return _c +} + +// Types provides a mock function with no fields +func (_m *MockFactory) Types() []v0alpha1.RepositoryType { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for Types") + } + + var r0 []v0alpha1.RepositoryType + if rf, ok := ret.Get(0).(func() []v0alpha1.RepositoryType); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]v0alpha1.RepositoryType) + } + } + + return r0 +} + +// MockFactory_Types_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Types' +type MockFactory_Types_Call struct { + *mock.Call +} + +// Types is a helper method to define mock.On call +func (_e *MockFactory_Expecter) Types() *MockFactory_Types_Call { + return &MockFactory_Types_Call{Call: _e.mock.On("Types")} +} + +func (_c *MockFactory_Types_Call) Run(run func()) *MockFactory_Types_Call { + _c.Call.Run(func(args mock.Arguments) { + run() + }) + return _c +} + +func (_c *MockFactory_Types_Call) Return(_a0 []v0alpha1.RepositoryType) *MockFactory_Types_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockFactory_Types_Call) RunAndReturn(run func() []v0alpha1.RepositoryType) *MockFactory_Types_Call { + _c.Call.Return(run) + return _c +} + +// NewMockFactory creates a new instance of MockFactory. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewMockFactory(t interface { + mock.TestingT + Cleanup(func()) +}) *MockFactory { + mock := &MockFactory{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/apps/provisioning/pkg/repository/finalizers.go b/apps/provisioning/pkg/repository/finalizers.go index ea8ae2a9021..9b674e21242 100644 --- a/apps/provisioning/pkg/repository/finalizers.go +++ b/apps/provisioning/pkg/repository/finalizers.go @@ -8,3 +8,9 @@ const ReleaseOrphanResourcesFinalizer = "release-orphan-resources" // CleanFinalizer calls the "OnDelete" function for resource const CleanFinalizer = "cleanup" + +var SupportedFinalizers = []string{ + RemoveOrphanResourcesFinalizer, + ReleaseOrphanResourcesFinalizer, + CleanFinalizer, +} diff --git a/apps/provisioning/pkg/repository/github/webhook.go b/apps/provisioning/pkg/repository/github/webhook.go index 080030253e0..3c37b00cba7 100644 --- a/apps/provisioning/pkg/repository/github/webhook.go +++ b/apps/provisioning/pkg/repository/github/webhook.go @@ -273,9 +273,14 @@ func (r *githubWebhookRepository) deleteWebhook(ctx context.Context) error { id := r.config.Status.Webhook.ID - if err := r.gh.DeleteWebhook(ctx, r.owner, r.repo, id); err != nil { + err := r.gh.DeleteWebhook(ctx, r.owner, r.repo, id) + if err != nil && !errors.Is(err, ErrResourceNotFound) { return fmt.Errorf("delete webhook: %w", err) } + if errors.Is(err, ErrResourceNotFound) { + logger.Info("webhook does not exist", "url", r.config.Status.Webhook.URL, "id", id) + return nil + } logger.Info("webhook deleted", "url", r.config.Status.Webhook.URL, "id", id) return nil diff --git a/apps/provisioning/pkg/repository/github/webhook_test.go b/apps/provisioning/pkg/repository/github/webhook_test.go index 24127f7359d..5021623fe90 100644 --- a/apps/provisioning/pkg/repository/github/webhook_test.go +++ b/apps/provisioning/pkg/repository/github/webhook_test.go @@ -1539,6 +1539,32 @@ func TestGitHubRepository_OnDelete(t *testing.T) { webhookURL: "https://example.com/webhook", expectedError: nil, }, + { + name: "webhook not found during deletion", + setupMock: func(m *MockClient) { + m.On("DeleteWebhook", mock.Anything, "grafana", "grafana", int64(123)). + Return(ErrResourceNotFound) + }, + config: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + }, + Spec: provisioning.RepositorySpec{ + GitHub: &provisioning.GitHubRepositoryConfig{ + Branch: "main", + }, + }, + Status: provisioning.RepositoryStatus{ + Webhook: &provisioning.WebhookStatus{ + ID: 123, + URL: "https://example.com/webhook", + }, + }, + }, + webhookURL: "https://example.com/webhook", + // We don't return an error if the webhook is already gone + expectedError: nil, + }, { name: "no webhook URL provided", setupMock: func(_ *MockClient) {}, diff --git a/apps/provisioning/pkg/repository/test.go b/apps/provisioning/pkg/repository/test.go index b6307237a0a..4c81013c477 100644 --- a/apps/provisioning/pkg/repository/test.go +++ b/apps/provisioning/pkg/repository/test.go @@ -124,6 +124,18 @@ func ValidateRepository(repo Repository) field.ErrorList { ) } + for _, f := range cfg.Finalizers { + if !slices.Contains(SupportedFinalizers, f) { + list = append(list, + field.Invalid( + field.NewPath("medatada", "finalizers"), + cfg.Finalizers, + fmt.Sprintf("unknown finalizer: %s", f), + ), + ) + } + } + return list } diff --git a/apps/provisioning/pkg/repository/test_test.go b/apps/provisioning/pkg/repository/test_test.go index 0a35c024ca4..752f28bb979 100644 --- a/apps/provisioning/pkg/repository/test_test.go +++ b/apps/provisioning/pkg/repository/test_test.go @@ -257,6 +257,28 @@ func TestValidateRepository(t *testing.T) { require.Contains(t, errors.ToAggregate().Error(), "cannot have both remove and release orphan resources finalizers") }, }, + { + name: "invalid finalizer in the list", + repository: func() *MockRepository { + m := NewMockRepository(t) + m.On("Config").Return(&provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Finalizers: []string{CleanFinalizer, "invalid-finalizer", RemoveOrphanResourcesFinalizer}, + }, + Spec: provisioning.RepositorySpec{ + Title: "Test Repo", + Type: provisioning.GitHubRepositoryType, + Workflows: []provisioning.Workflow{provisioning.WriteWorkflow}, + }, + }) + m.On("Validate").Return(field.ErrorList{}) + return m + }(), + expectedErrs: 1, + validateError: func(t *testing.T, errors field.ErrorList) { + require.Contains(t, errors.ToAggregate().Error(), "unknown finalizer: invalid-finalizer") + }, + }, } for _, tt := range tests { diff --git a/pkg/apis/featuretoggle/v0alpha1/zz_generated.openapi.go b/pkg/apis/featuretoggle/v0alpha1/zz_generated.openapi.go deleted file mode 100644 index d713284519b..00000000000 --- a/pkg/apis/featuretoggle/v0alpha1/zz_generated.openapi.go +++ /dev/null @@ -1,445 +0,0 @@ -//go:build !ignore_autogenerated -// +build !ignore_autogenerated - -// SPDX-License-Identifier: AGPL-3.0-only - -// Code generated by openapi-gen. DO NOT EDIT. - -package v0alpha1 - -import ( - common "k8s.io/kube-openapi/pkg/common" - spec "k8s.io/kube-openapi/pkg/validation/spec" -) - -func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { - return map[string]common.OpenAPIDefinition{ - "github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.Feature": schema_pkg_apis_featuretoggle_v0alpha1_Feature(ref), - "github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.FeatureList": schema_pkg_apis_featuretoggle_v0alpha1_FeatureList(ref), - "github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.FeatureSpec": schema_pkg_apis_featuretoggle_v0alpha1_FeatureSpec(ref), - "github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.FeatureToggles": schema_pkg_apis_featuretoggle_v0alpha1_FeatureToggles(ref), - "github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.FeatureTogglesList": schema_pkg_apis_featuretoggle_v0alpha1_FeatureTogglesList(ref), - "github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.ResolvedToggleState": schema_pkg_apis_featuretoggle_v0alpha1_ResolvedToggleState(ref), - "github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.ToggleStatus": schema_pkg_apis_featuretoggle_v0alpha1_ToggleStatus(ref), - } -} - -func schema_pkg_apis_featuretoggle_v0alpha1_Feature(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Description: "Feature represents a feature in development and information about that feature It does *not* know the status, only defines properties about the feature itself", - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "kind": { - SchemaProps: spec.SchemaProps{ - Description: "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds", - Type: []string{"string"}, - Format: "", - }, - }, - "apiVersion": { - SchemaProps: spec.SchemaProps{ - Description: "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources", - Type: []string{"string"}, - Format: "", - }, - }, - "metadata": { - SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta"), - }, - }, - "spec": { - SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.FeatureSpec"), - }, - }, - }, - }, - }, - Dependencies: []string{ - "github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.FeatureSpec", "k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta"}, - } -} - -func schema_pkg_apis_featuretoggle_v0alpha1_FeatureList(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "kind": { - SchemaProps: spec.SchemaProps{ - Description: "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds", - Type: []string{"string"}, - Format: "", - }, - }, - "apiVersion": { - SchemaProps: spec.SchemaProps{ - Description: "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources", - Type: []string{"string"}, - Format: "", - }, - }, - "metadata": { - SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.ListMeta"), - }, - }, - "items": { - SchemaProps: spec.SchemaProps{ - Type: []string{"array"}, - Items: &spec.SchemaOrArray{ - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.Feature"), - }, - }, - }, - }, - }, - }, - Required: []string{"items"}, - }, - }, - Dependencies: []string{ - "github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.Feature", "k8s.io/apimachinery/pkg/apis/meta/v1.ListMeta"}, - } -} - -func schema_pkg_apis_featuretoggle_v0alpha1_FeatureSpec(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "description": { - SchemaProps: spec.SchemaProps{ - Description: "The feature description", - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - "stage": { - SchemaProps: spec.SchemaProps{ - Description: "Indicates the features level of stability", - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - "codeowner": { - SchemaProps: spec.SchemaProps{ - Description: "The team who owns this feature development", - Type: []string{"string"}, - Format: "", - }, - }, - "enabledVersion": { - SchemaProps: spec.SchemaProps{ - Description: "Enabled by default for version >=", - Type: []string{"string"}, - Format: "", - }, - }, - "requiresDevMode": { - SchemaProps: spec.SchemaProps{ - Description: "Must be run using in development mode (early dev)", - Type: []string{"boolean"}, - Format: "", - }, - }, - "frontend": { - SchemaProps: spec.SchemaProps{ - Description: "The flab behavior only effects frontend -- it is not used in the backend", - Type: []string{"boolean"}, - Format: "", - }, - }, - "requiresRestart": { - SchemaProps: spec.SchemaProps{ - Description: "The flag is used at startup, so any change requires a restart", - Type: []string{"boolean"}, - Format: "", - }, - }, - "allowSelfServe": { - SchemaProps: spec.SchemaProps{ - Description: "Allow cloud users to set the values in UI", - Type: []string{"boolean"}, - Format: "", - }, - }, - "hideFromAdminPage": { - SchemaProps: spec.SchemaProps{ - Description: "Do not show the value in the UI", - Type: []string{"boolean"}, - Format: "", - }, - }, - "hideFromDocs": { - SchemaProps: spec.SchemaProps{ - Description: "Do not show the value in docs", - Type: []string{"boolean"}, - Format: "", - }, - }, - "expression": { - SchemaProps: spec.SchemaProps{ - Description: "Expression to determine if the flag is enabled by default -- can only be \"true\" for toggles that are public preview, generally available, or deprecated", - Type: []string{"string"}, - Format: "", - }, - }, - }, - Required: []string{"description", "stage"}, - }, - }, - } -} - -func schema_pkg_apis_featuretoggle_v0alpha1_FeatureToggles(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Description: "FeatureToggles define the feature state", - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "kind": { - SchemaProps: spec.SchemaProps{ - Description: "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds", - Type: []string{"string"}, - Format: "", - }, - }, - "apiVersion": { - SchemaProps: spec.SchemaProps{ - Description: "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources", - Type: []string{"string"}, - Format: "", - }, - }, - "metadata": { - SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta"), - }, - }, - "spec": { - SchemaProps: spec.SchemaProps{ - Description: "The configured toggles. Note this may include unknown fields", - Type: []string{"object"}, - AdditionalProperties: &spec.SchemaOrBool{ - Allows: true, - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Default: false, - Type: []string{"boolean"}, - Format: "", - }, - }, - }, - }, - }, - }, - Required: []string{"spec"}, - }, - }, - Dependencies: []string{ - "k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta"}, - } -} - -func schema_pkg_apis_featuretoggle_v0alpha1_FeatureTogglesList(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "kind": { - SchemaProps: spec.SchemaProps{ - Description: "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds", - Type: []string{"string"}, - Format: "", - }, - }, - "apiVersion": { - SchemaProps: spec.SchemaProps{ - Description: "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources", - Type: []string{"string"}, - Format: "", - }, - }, - "metadata": { - SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.ListMeta"), - }, - }, - "items": { - SchemaProps: spec.SchemaProps{ - Type: []string{"array"}, - Items: &spec.SchemaOrArray{ - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.FeatureToggles"), - }, - }, - }, - }, - }, - }, - Required: []string{"items"}, - }, - }, - Dependencies: []string{ - "github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.FeatureToggles", "k8s.io/apimachinery/pkg/apis/meta/v1.ListMeta"}, - } -} - -func schema_pkg_apis_featuretoggle_v0alpha1_ResolvedToggleState(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "kind": { - SchemaProps: spec.SchemaProps{ - Description: "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds", - Type: []string{"string"}, - Format: "", - }, - }, - "apiVersion": { - SchemaProps: spec.SchemaProps{ - Description: "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources", - Type: []string{"string"}, - Format: "", - }, - }, - "allowEditing": { - SchemaProps: spec.SchemaProps{ - Description: "The user is allowed to edit feature toggles on this system", - Type: []string{"boolean"}, - Format: "", - }, - }, - "restartRequired": { - SchemaProps: spec.SchemaProps{ - Description: "The system has changes that require still require a restart", - Type: []string{"boolean"}, - Format: "", - }, - }, - "enabled": { - SchemaProps: spec.SchemaProps{ - Description: "The currently enabled flags", - Type: []string{"object"}, - AdditionalProperties: &spec.SchemaOrBool{ - Allows: true, - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Default: false, - Type: []string{"boolean"}, - Format: "", - }, - }, - }, - }, - }, - "toggles": { - SchemaProps: spec.SchemaProps{ - Description: "Details on the current status", - Type: []string{"array"}, - Items: &spec.SchemaOrArray{ - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.ToggleStatus"), - }, - }, - }, - }, - }, - }, - }, - }, - Dependencies: []string{ - "github.com/grafana/grafana/pkg/apis/featuretoggle/v0alpha1.ToggleStatus"}, - } -} - -func schema_pkg_apis_featuretoggle_v0alpha1_ToggleStatus(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "name": { - SchemaProps: spec.SchemaProps{ - Description: "The feature toggle name", - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - "description": { - SchemaProps: spec.SchemaProps{ - Description: "The flag description", - Type: []string{"string"}, - Format: "", - }, - }, - "stage": { - SchemaProps: spec.SchemaProps{ - Description: "The feature toggle stage", - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - "enabled": { - SchemaProps: spec.SchemaProps{ - Description: "Is the flag enabled", - Default: false, - Type: []string{"boolean"}, - Format: "", - }, - }, - "writeable": { - SchemaProps: spec.SchemaProps{ - Description: "Can this flag be updated", - Default: false, - Type: []string{"boolean"}, - Format: "", - }, - }, - "source": { - SchemaProps: spec.SchemaProps{ - Description: "Where was the value configured eg: startup | tenant|org | user | browser missing means default", - Ref: ref("github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1.ObjectReference"), - }, - }, - "warning": { - SchemaProps: spec.SchemaProps{ - Description: "eg: unknown flag", - Type: []string{"string"}, - Format: "", - }, - }, - }, - Required: []string{"name", "stage", "enabled", "writeable"}, - }, - }, - Dependencies: []string{ - "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1.ObjectReference"}, - } -} diff --git a/pkg/registry/apis/provisioning/controller/finalizer_mock.go b/pkg/registry/apis/provisioning/controller/finalizer_mock.go new file mode 100644 index 00000000000..a918ba35b52 --- /dev/null +++ b/pkg/registry/apis/provisioning/controller/finalizer_mock.go @@ -0,0 +1,85 @@ +// Code generated by mockery v2.53.4. DO NOT EDIT. + +package controller + +import ( + context "context" + + repository "github.com/grafana/grafana/apps/provisioning/pkg/repository" + mock "github.com/stretchr/testify/mock" +) + +// MockFinalizerProcessor is an autogenerated mock type for the finalizerProcessor type +type MockFinalizerProcessor struct { + mock.Mock +} + +type MockFinalizerProcessor_Expecter struct { + mock *mock.Mock +} + +func (_m *MockFinalizerProcessor) EXPECT() *MockFinalizerProcessor_Expecter { + return &MockFinalizerProcessor_Expecter{mock: &_m.Mock} +} + +// process provides a mock function with given fields: ctx, repo, finalizers +func (_m *MockFinalizerProcessor) process(ctx context.Context, repo repository.Repository, finalizers []string) error { + ret := _m.Called(ctx, repo, finalizers) + + if len(ret) == 0 { + panic("no return value specified for process") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, repository.Repository, []string) error); ok { + r0 = rf(ctx, repo, finalizers) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MockFinalizerProcessor_process_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'process' +type MockFinalizerProcessor_process_Call struct { + *mock.Call +} + +// process is a helper method to define mock.On call +// - ctx context.Context +// - repo repository.Repository +// - finalizers []string +func (_e *MockFinalizerProcessor_Expecter) process(ctx interface{}, repo interface{}, finalizers interface{}) *MockFinalizerProcessor_process_Call { + return &MockFinalizerProcessor_process_Call{Call: _e.mock.On("process", ctx, repo, finalizers)} +} + +func (_c *MockFinalizerProcessor_process_Call) Run(run func(ctx context.Context, repo repository.Repository, finalizers []string)) *MockFinalizerProcessor_process_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(repository.Repository), args[2].([]string)) + }) + return _c +} + +func (_c *MockFinalizerProcessor_process_Call) Return(_a0 error) *MockFinalizerProcessor_process_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockFinalizerProcessor_process_Call) RunAndReturn(run func(context.Context, repository.Repository, []string) error) *MockFinalizerProcessor_process_Call { + _c.Call.Return(run) + return _c +} + +// NewMockFinalizerProcessor creates a new instance of MockFinalizerProcessor. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewMockFinalizerProcessor(t interface { + mock.TestingT + Cleanup(func()) +}) *MockFinalizerProcessor { + mock := &MockFinalizerProcessor{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/registry/apis/provisioning/controller/finalizers.go b/pkg/registry/apis/provisioning/controller/finalizers.go index 952d11bf0f8..18f00c17c40 100644 --- a/pkg/registry/apis/provisioning/controller/finalizers.go +++ b/pkg/registry/apis/provisioning/controller/finalizers.go @@ -3,6 +3,8 @@ package controller import ( "context" "encoding/json" + "fmt" + "slices" "sort" "strings" "time" @@ -32,8 +34,18 @@ func (f *finalizer) process(ctx context.Context, finalizers []string, ) error { logger := logging.FromContext(ctx) + logger.Info("process finalizers", "finalizers", finalizers) - for _, finalizer := range finalizers { + orderedFinalizers := [3]string{ + repository.CleanFinalizer, + repository.ReleaseOrphanResourcesFinalizer, + repository.RemoveOrphanResourcesFinalizer} + + for _, finalizer := range orderedFinalizers { + if !slices.Contains(finalizers, finalizer) { + continue + } + logger.Info("running finalizer", "finalizer", finalizer) var err error var count int start := time.Now() @@ -42,44 +54,33 @@ func (f *finalizer) process(ctx context.Context, switch finalizer { case repository.CleanFinalizer: // NOTE: the controller loop will never get run unless a finalizer is set + logger.Info("running cleanup finalizer") hooks, ok := repo.(repository.Hooks) if ok { if err = hooks.OnDelete(ctx); err != nil { - logger.Warn("Error running deletion hooks", "err", err) + err = fmt.Errorf("execute deletion hooks: %w", err) outcome = metricutils.ErrorOutcome } } case repository.ReleaseOrphanResourcesFinalizer: - count, err = f.processExistingItems(ctx, repo.Config(), - func(client dynamic.ResourceInterface, item *provisioning.ResourceListItem) error { - patchAnnotations, err := getPatchedAnnotations(item) - if err != nil { - return err - } - - _, err = client.Patch( - ctx, item.Name, types.JSONPatchType, patchAnnotations, v1.PatchOptions{}, - ) - return err - }) + logger.Info("releasing orphan resources") + count, err = f.processExistingItems(ctx, repo.Config(), f.releaseResources(ctx, logger)) if err != nil { + err = fmt.Errorf("release resources: %w", err) outcome = metricutils.ErrorOutcome - logger.Warn("Error processing release orphan resources finalizer", "err", err) } case repository.RemoveOrphanResourcesFinalizer: - count, err = f.processExistingItems(ctx, repo.Config(), - func(client dynamic.ResourceInterface, item *provisioning.ResourceListItem) error { - return client.Delete(ctx, item.Name, v1.DeleteOptions{}) - }) + logger.Info("removing orphan resources") + count, err = f.processExistingItems(ctx, repo.Config(), f.removeResources(ctx, logger)) if err != nil { + err = fmt.Errorf("remove resources: %w", err) outcome = metricutils.ErrorOutcome - logger.Warn("Error processing remove orphan resources finalizer", "err", err) } default: - logger.Warn("skipping unknown finalizer", "finalizer", finalizer) + logger.Error("skipping unknown finalizer", "finalizer", finalizer) continue } @@ -106,36 +107,74 @@ func (f *finalizer) processExistingItems( items, err := f.lister.List(ctx, repo.Namespace, repo.Name) if err != nil { - logger.Warn("error listing resources", "error", err) + logger.Error("error listing resources", "error", err) return 0, err } // Safe deletion order sortResourceListForDeletion(items) count := 0 - errors := 0 for _, item := range items.Items { res, _, err := clients.ForResource(ctx, schema.GroupVersionResource{ Group: item.Group, Resource: item.Resource, }) + logger.Error("error getting client for resource", "resource", item.Resource, "error", err) if err != nil { return count, err } err = cb(res, &item) if err != nil { - logger.Warn("error processing item", "name", item.Name, "error", err) - errors++ + logger.Error("error processing item", "name", item.Name, "error", err) + return count, fmt.Errorf("processing item: %w", err) } else { count++ } } - logger.Info("processed orphan items", "items", count, "errors", errors) + logger.Info("processed orphan items", "items", count) return count, nil } +func (f *finalizer) releaseResources( + ctx context.Context, logger logging.Logger, +) func(client dynamic.ResourceInterface, item *provisioning.ResourceListItem) error { + return func(client dynamic.ResourceInterface, item *provisioning.ResourceListItem) error { + logger.Info("release resource", + "name", item.Name, + "group", item.Group, + "resource", item.Resource, + ) + + patchAnnotations, err := getPatchedAnnotations(item) + if err != nil { + return fmt.Errorf("get patched annotations: %w", err) + } + + _, err = client.Patch( + ctx, item.Name, types.JSONPatchType, patchAnnotations, v1.PatchOptions{}, + ) + if err != nil { + return fmt.Errorf("patch resource to release ownership: %w", err) + } + return nil + } +} + +func (f *finalizer) removeResources( + ctx context.Context, logger logging.Logger, +) func(client dynamic.ResourceInterface, item *provisioning.ResourceListItem) error { + return func(client dynamic.ResourceInterface, item *provisioning.ResourceListItem) error { + logger.Info("remove resource", + "name", item.Name, + "group", item.Group, + "resource", item.Resource, + ) + return client.Delete(ctx, item.Name, v1.DeleteOptions{}) + } +} + type jsonPatchOperation struct { Op string `json:"op"` Path string `json:"path"` diff --git a/pkg/registry/apis/provisioning/controller/finalizers_test.go b/pkg/registry/apis/provisioning/controller/finalizers_test.go index a44e595afbf..140363a0507 100644 --- a/pkg/registry/apis/provisioning/controller/finalizers_test.go +++ b/pkg/registry/apis/provisioning/controller/finalizers_test.go @@ -1,12 +1,508 @@ package controller import ( + "context" "testing" - provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + "github.com/grafana/grafana/apps/provisioning/pkg/repository" + "github.com/grafana/grafana/pkg/registry/apis/provisioning/resources" ) +var ( + _ dynamic.ResourceInterface = (*mockDynamicClient)(nil) + _ repository.Repository = (*mockRepo)(nil) + _ repository.Hooks = (*mockRepo)(nil) +) + +type mockDynamicClient struct { + deleteFunc func(ctx context.Context, name string, options metav1.DeleteOptions, subresources ...string) error + patchFunc func(ctx context.Context, name string, pt types.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*unstructured.Unstructured, error) +} + +func (m mockDynamicClient) Create(ctx context.Context, obj *unstructured.Unstructured, options metav1.CreateOptions, subresources ...string) (*unstructured.Unstructured, error) { + panic("not needed for testing") +} + +func (m mockDynamicClient) Update(ctx context.Context, obj *unstructured.Unstructured, options metav1.UpdateOptions, subresources ...string) (*unstructured.Unstructured, error) { + panic("not needed for testing") +} + +func (m mockDynamicClient) UpdateStatus(ctx context.Context, obj *unstructured.Unstructured, options metav1.UpdateOptions) (*unstructured.Unstructured, error) { + panic("not needed for testing") +} + +func (m mockDynamicClient) Delete(ctx context.Context, name string, options metav1.DeleteOptions, subresources ...string) error { + if m.deleteFunc != nil { + return m.deleteFunc(ctx, name, options, subresources...) + } + return nil +} + +func (m mockDynamicClient) DeleteCollection(ctx context.Context, options metav1.DeleteOptions, listOptions metav1.ListOptions) error { + panic("not needed for testing") +} + +func (m mockDynamicClient) Get(ctx context.Context, name string, options metav1.GetOptions, subresources ...string) (*unstructured.Unstructured, error) { + panic("not needed for testing") +} + +func (m mockDynamicClient) List(ctx context.Context, opts metav1.ListOptions) (*unstructured.UnstructuredList, error) { + panic("not needed for testing") +} + +func (m mockDynamicClient) Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { + panic("not needed for testing") +} + +func (m mockDynamicClient) Patch(ctx context.Context, name string, pt types.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*unstructured.Unstructured, error) { + if m.patchFunc != nil { + return m.patchFunc(ctx, name, pt, data, options, subresources...) + } + return nil, nil +} + +func (m mockDynamicClient) Apply(ctx context.Context, name string, obj *unstructured.Unstructured, options metav1.ApplyOptions, subresources ...string) (*unstructured.Unstructured, error) { + panic("not needed for testing") +} + +func (m mockDynamicClient) ApplyStatus(ctx context.Context, name string, obj *unstructured.Unstructured, options metav1.ApplyOptions) (*unstructured.Unstructured, error) { + panic("not needed for testing") +} + +type mockRepo struct { + name string + namespace string + onDeleteFunc func(ctx context.Context) error +} + +func (m mockRepo) OnCreate(ctx context.Context) ([]map[string]interface{}, error) { + panic("not needed for testing") +} + +func (m mockRepo) OnUpdate(ctx context.Context) ([]map[string]interface{}, error) { + panic("not needed for testing") +} + +func (m mockRepo) OnDelete(ctx context.Context) error { + if m.onDeleteFunc != nil { + return m.onDeleteFunc(ctx) + } + return nil +} + +func (m mockRepo) Config() *provisioning.Repository { + return &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: m.name, + Namespace: m.namespace, + }, + } +} + +func (m mockRepo) Validate() field.ErrorList { + panic("not needed for testing") +} + +func (m mockRepo) Test(ctx context.Context) (*provisioning.TestResults, error) { + panic("not needed for testing") +} + +func TestFinalizer_process(t *testing.T) { + testCases := []struct { + name string + lister resources.ResourceLister + clientFactory resources.ClientFactory + repo repository.Repository + finalizers []string + expectedErr string + }{ + { + name: "No finalizers", + lister: nil, + clientFactory: nil, + repo: nil, + finalizers: []string{}, + }, + { + name: "Successfully releases resources and cleanup hooks", + lister: func() resources.ResourceLister { + resourceLister := resources.NewMockResourceLister(t) + + resourceLister. + On("List", context.Background(), "default", "my-repo"). + Once(). + Return(&provisioning.ResourceList{ + Items: []provisioning.ResourceListItem{ + { + Group: "dashboard.grafana.app", + Resource: "dashboards", + Name: "my-dashboard", + }, + }, + }, nil) + + return resourceLister + }(), + clientFactory: func() resources.ClientFactory { + clientFactory := resources.NewMockClientFactory(t) + clients := resources.NewMockResourceClients(t) + client := &mockDynamicClient{ + patchFunc: func(ctx context.Context, name string, pt types.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*unstructured.Unstructured, error) { + return &unstructured.Unstructured{}, nil + }, + } + + clientFactory. + On("Clients", context.Background(), "default"). + Once(). + Return(clients, nil) + + clients. + On("ForResource", context.Background(), schema.GroupVersionResource{ + Group: "dashboard.grafana.app", + Resource: "dashboards", + }). + Once(). + Return(client, schema.GroupVersionKind{}, nil) + + return clientFactory + }(), + repo: mockRepo{ + name: "my-repo", + namespace: "default", + onDeleteFunc: func(ctx context.Context) error { + return nil + }, + }, + finalizers: []string{ + repository.ReleaseOrphanResourcesFinalizer, + repository.CleanFinalizer, + }, + }, + { + name: "Successfully removes resources and cleanup hooks", + lister: func() resources.ResourceLister { + resourceLister := resources.NewMockResourceLister(t) + + resourceLister. + On("List", context.Background(), "default", "my-repo"). + Once(). + Return(&provisioning.ResourceList{ + Items: []provisioning.ResourceListItem{ + { + Group: "dashboard.grafana.app", + Resource: "dashboards", + Name: "my-dashboard", + }, + }, + }, nil) + + return resourceLister + }(), + clientFactory: func() resources.ClientFactory { + clientFactory := resources.NewMockClientFactory(t) + clients := resources.NewMockResourceClients(t) + client := &mockDynamicClient{ + deleteFunc: func(ctx context.Context, name string, options metav1.DeleteOptions, subresources ...string) error { + return nil + }, + } + + clientFactory. + On("Clients", context.Background(), "default"). + Once(). + Return(clients, nil) + + clients. + On("ForResource", context.Background(), schema.GroupVersionResource{ + Group: "dashboard.grafana.app", + Resource: "dashboards", + }). + Once(). + Return(client, schema.GroupVersionKind{}, nil) + + return clientFactory + }(), + repo: mockRepo{ + name: "my-repo", + namespace: "default", + onDeleteFunc: func(ctx context.Context) error { + return nil + }, + }, + finalizers: []string{ + repository.RemoveOrphanResourcesFinalizer, + repository.CleanFinalizer, + }, + }, + { + name: "Issue getting the namespace clients", + lister: nil, + clientFactory: func() resources.ClientFactory { + clientFactory := resources.NewMockClientFactory(t) + + clientFactory. + On("Clients", context.Background(), "default"). + Once(). + Return(nil, assert.AnError) + + return clientFactory + }(), + repo: mockRepo{ + name: "my-repo", + namespace: "default", + }, + finalizers: []string{ + repository.RemoveOrphanResourcesFinalizer, + repository.CleanFinalizer, + }, + expectedErr: "remove resources: " + assert.AnError.Error(), + }, + { + name: "Issue listing items", + lister: func() resources.ResourceLister { + resourceLister := resources.NewMockResourceLister(t) + + resourceLister. + On("List", context.Background(), "default", "my-repo"). + Once(). + Return(nil, assert.AnError) + + return resourceLister + }(), + clientFactory: func() resources.ClientFactory { + clientFactory := resources.NewMockClientFactory(t) + clients := resources.NewMockResourceClients(t) + + clientFactory. + On("Clients", context.Background(), "default"). + Once(). + Return(clients, nil) + + return clientFactory + }(), + repo: mockRepo{ + name: "my-repo", + namespace: "default", + }, + finalizers: []string{ + repository.RemoveOrphanResourcesFinalizer, + repository.CleanFinalizer, + }, + expectedErr: "remove resources: " + assert.AnError.Error(), + }, + { + name: "Issue getting client for resource", + lister: func() resources.ResourceLister { + resourceLister := resources.NewMockResourceLister(t) + + resourceLister. + On("List", context.Background(), "default", "my-repo"). + Once(). + Return(&provisioning.ResourceList{ + Items: []provisioning.ResourceListItem{ + { + Group: "dashboard.grafana.app", + Resource: "dashboards", + Name: "my-dashboard", + }, + }, + }, nil) + + return resourceLister + }(), + clientFactory: func() resources.ClientFactory { + clientFactory := resources.NewMockClientFactory(t) + clients := resources.NewMockResourceClients(t) + + clientFactory. + On("Clients", context.Background(), "default"). + Once(). + Return(clients, nil) + + clients. + On("ForResource", context.Background(), schema.GroupVersionResource{ + Group: "dashboard.grafana.app", + Resource: "dashboards", + }). + Once(). + Return(nil, schema.GroupVersionKind{}, assert.AnError) + + return clientFactory + }(), + repo: mockRepo{ + name: "my-repo", + namespace: "default", + }, + finalizers: []string{ + repository.RemoveOrphanResourcesFinalizer, + repository.CleanFinalizer, + }, + expectedErr: "remove resources: " + assert.AnError.Error(), + }, + { + name: "Error deleting items", + lister: func() resources.ResourceLister { + resourceLister := resources.NewMockResourceLister(t) + + resourceLister. + On("List", context.Background(), "default", "my-repo"). + Once(). + Return(&provisioning.ResourceList{ + Items: []provisioning.ResourceListItem{ + { + Group: "dashboard.grafana.app", + Resource: "dashboards", + Name: "my-dashboard", + }, + }, + }, nil) + + return resourceLister + }(), + clientFactory: func() resources.ClientFactory { + clientFactory := resources.NewMockClientFactory(t) + clients := resources.NewMockResourceClients(t) + client := &mockDynamicClient{ + deleteFunc: func(ctx context.Context, name string, options metav1.DeleteOptions, subresources ...string) error { + return assert.AnError + }, + } + + clientFactory. + On("Clients", context.Background(), "default"). + Once(). + Return(clients, nil) + + clients. + On("ForResource", context.Background(), schema.GroupVersionResource{ + Group: "dashboard.grafana.app", + Resource: "dashboards", + }). + Once(). + Return(client, schema.GroupVersionKind{}, nil) + + return clientFactory + }(), + repo: mockRepo{ + name: "my-repo", + namespace: "default", + onDeleteFunc: func(ctx context.Context) error { + return nil + }, + }, + finalizers: []string{ + repository.RemoveOrphanResourcesFinalizer, + repository.CleanFinalizer, + }, + expectedErr: "remove resources", + }, + { + name: "Error releasing items", + lister: func() resources.ResourceLister { + resourceLister := resources.NewMockResourceLister(t) + + resourceLister. + On("List", context.Background(), "default", "my-repo"). + Once(). + Return(&provisioning.ResourceList{ + Items: []provisioning.ResourceListItem{ + { + Group: "dashboard.grafana.app", + Resource: "dashboards", + Name: "my-dashboard", + }, + }, + }, nil) + + return resourceLister + }(), + clientFactory: func() resources.ClientFactory { + clientFactory := resources.NewMockClientFactory(t) + clients := resources.NewMockResourceClients(t) + client := &mockDynamicClient{ + patchFunc: func(ctx context.Context, name string, pt types.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*unstructured.Unstructured, error) { + return nil, assert.AnError + }, + } + + clientFactory. + On("Clients", context.Background(), "default"). + Once(). + Return(clients, nil) + + clients. + On("ForResource", context.Background(), schema.GroupVersionResource{ + Group: "dashboard.grafana.app", + Resource: "dashboards", + }). + Once(). + Return(client, schema.GroupVersionKind{}, nil) + + return clientFactory + }(), + repo: mockRepo{ + name: "my-repo", + namespace: "default", + onDeleteFunc: func(ctx context.Context) error { + return nil + }, + }, + finalizers: []string{ + repository.ReleaseOrphanResourcesFinalizer, + repository.CleanFinalizer, + }, + expectedErr: "release resources", + }, + { + name: "Error deleting hooks", + lister: nil, + clientFactory: nil, + repo: mockRepo{ + name: "my-repo", + namespace: "default", + onDeleteFunc: func(ctx context.Context) error { + return assert.AnError + }, + }, + finalizers: []string{ + repository.RemoveOrphanResourcesFinalizer, + repository.CleanFinalizer, + }, + expectedErr: "execute deletion hooks: " + assert.AnError.Error(), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + metrics := registerFinalizerMetrics(prometheus.NewRegistry()) + f := &finalizer{ + lister: tc.lister, + clientFactory: tc.clientFactory, + metrics: &metrics, + } + err := f.process(context.Background(), tc.repo, tc.finalizers) + if tc.expectedErr == "" { + assert.NoError(t, err) + } else { + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErr) + } + }) + } +} + func TestSortResourceListForDeletion(t *testing.T) { testCases := []struct { name string diff --git a/pkg/registry/apis/provisioning/controller/repository.go b/pkg/registry/apis/provisioning/controller/repository.go index ccda76e1722..b8301f784df 100644 --- a/pkg/registry/apis/provisioning/controller/repository.go +++ b/pkg/registry/apis/provisioning/controller/repository.go @@ -41,6 +41,11 @@ type queueItem struct { attempts int } +//go:generate mockery --name finalizerProcessor --structname MockFinalizerProcessor --inpackage --filename finalizer_mock.go --with-expecter +type finalizerProcessor interface { + process(ctx context.Context, repo repository.Repository, finalizers []string) error +} + // RepositoryController controls how and when CRD is established. type RepositoryController struct { client client.ProvisioningV0alpha1Interface @@ -50,7 +55,7 @@ type RepositoryController struct { dualwrite dualwrite.Service jobs jobs.Queue - finalizer *finalizer + finalizer finalizerProcessor statusPatcher StatusPatcher repoFactory repository.Factory @@ -223,12 +228,15 @@ func (rc *RepositoryController) handleDelete(ctx context.Context, obj *provision if len(obj.Finalizers) > 0 { repo, err := rc.repoFactory.Build(ctx, obj) if err != nil { - logger.Warn("unable to get repository for cleanup") - } else { - err := rc.finalizer.process(ctx, repo, obj.Finalizers) - if err != nil { - logger.Warn("error running finalizer", "err", err) + return fmt.Errorf("create repository from configuration: %w", err) + } + + err = rc.finalizer.process(ctx, repo, obj.Finalizers) + if err != nil { + if statusErr := rc.updateDeleteStatus(ctx, obj, fmt.Errorf("remove finalizers: %w", err)); statusErr != nil { + logger.Error("failed to update repository status after finalizer removal error", "error", statusErr) } + return fmt.Errorf("process finalizers: %w", err) } // remove the finalizers @@ -238,12 +246,27 @@ func (rc *RepositoryController) handleDelete(ctx context.Context, obj *provision ]`), v1.PatchOptions{ FieldManager: "provisioning-controller", }) - return err // delete will be called again + if err != nil { + return fmt.Errorf("remove finalizers: %w", err) + } + return nil + } else { + logger.Info("no finalizers to process") } return nil } +func (rc *RepositoryController) updateDeleteStatus(ctx context.Context, obj *provisioning.Repository, err error) error { + logger := logging.FromContext(ctx) + logger.Info("updating repository status with deletion error", "error", err.Error()) + return rc.statusPatcher.Patch(ctx, obj, map[string]interface{}{ + "op": "replace", + "path": "/status/deleteError", + "value": err.Error(), + }) +} + func (rc *RepositoryController) shouldResync(obj *provisioning.Repository) bool { // don't trigger resync if a sync was never started if obj.Status.Sync.Finished == 0 && obj.Status.Sync.State == "" { diff --git a/pkg/registry/apis/provisioning/controller/repository_test.go b/pkg/registry/apis/provisioning/controller/repository_test.go new file mode 100644 index 00000000000..69f4275fe9e --- /dev/null +++ b/pkg/registry/apis/provisioning/controller/repository_test.go @@ -0,0 +1,305 @@ +package controller + +import ( + "context" + "testing" + + "github.com/grafana/grafana/pkg/registry/apis/provisioning/controller/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/rest" + + provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" + provisioningv0alpha1 "github.com/grafana/grafana/apps/provisioning/pkg/generated/applyconfiguration/provisioning/v0alpha1" + client "github.com/grafana/grafana/apps/provisioning/pkg/generated/clientset/versioned/typed/provisioning/v0alpha1" + "github.com/grafana/grafana/apps/provisioning/pkg/repository" +) + +type mockProvisioningV0alpha1Interface struct { + repositoriesFunc func(namespace string) client.RepositoryInterface +} + +func (m mockProvisioningV0alpha1Interface) RESTClient() rest.Interface { + panic("not needed for testing") +} + +func (m mockProvisioningV0alpha1Interface) HistoricJobs(namespace string) client.HistoricJobInterface { + panic("not needed for testing") +} + +func (m mockProvisioningV0alpha1Interface) Jobs(namespace string) client.JobInterface { + panic("not needed for testing") +} + +func (m mockProvisioningV0alpha1Interface) Repositories(namespace string) client.RepositoryInterface { + if m.repositoriesFunc != nil { + return m.repositoriesFunc(namespace) + } + return nil +} + +type mockRepoInterface struct { + patchFunc func(ctx context.Context, name string, pt types.PatchType, data []byte, opts metav1.PatchOptions, subresources ...string) (result *provisioning.Repository, err error) +} + +func (m mockRepoInterface) Create(ctx context.Context, repository *provisioning.Repository, opts metav1.CreateOptions) (*provisioning.Repository, error) { + panic("not needed for testing") +} + +func (m mockRepoInterface) Update(ctx context.Context, repository *provisioning.Repository, opts metav1.UpdateOptions) (*provisioning.Repository, error) { + panic("not needed for testing") +} + +func (m mockRepoInterface) UpdateStatus(ctx context.Context, repository *provisioning.Repository, opts metav1.UpdateOptions) (*provisioning.Repository, error) { + panic("not needed for testing") +} + +func (m mockRepoInterface) Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error { + panic("not needed for testing") +} + +func (m mockRepoInterface) DeleteCollection(ctx context.Context, opts metav1.DeleteOptions, listOpts metav1.ListOptions) error { + panic("not needed for testing") +} + +func (m mockRepoInterface) Get(ctx context.Context, name string, opts metav1.GetOptions) (*provisioning.Repository, error) { + panic("not needed for testing") +} + +func (m mockRepoInterface) List(ctx context.Context, opts metav1.ListOptions) (*provisioning.RepositoryList, error) { + panic("not needed for testing") +} + +func (m mockRepoInterface) Watch(ctx context.Context, opts metav1.ListOptions) (watch.Interface, error) { + panic("not needed for testing") +} + +func (m mockRepoInterface) Patch(ctx context.Context, name string, pt types.PatchType, data []byte, opts metav1.PatchOptions, subresources ...string) (result *provisioning.Repository, err error) { + if m.patchFunc != nil { + return m.patchFunc(ctx, name, pt, data, opts, subresources...) + } + return nil, nil +} + +func (m mockRepoInterface) Apply(ctx context.Context, repository *provisioningv0alpha1.RepositoryApplyConfiguration, opts metav1.ApplyOptions) (result *provisioning.Repository, err error) { + panic("not needed for testing") +} + +func (m mockRepoInterface) ApplyStatus(ctx context.Context, repository *provisioningv0alpha1.RepositoryApplyConfiguration, opts metav1.ApplyOptions) (result *provisioning.Repository, err error) { + panic("not needed for testing") +} + +var ( + _ client.ProvisioningV0alpha1Interface = (*mockProvisioningV0alpha1Interface)(nil) + _ client.RepositoryInterface = (*mockRepoInterface)(nil) +) + +func TestRepositoryController_handleDelete(t *testing.T) { + testCases := []struct { + name string + repoFactory repository.Factory + finalizer finalizerProcessor + client client.ProvisioningV0alpha1Interface + statusPatcher StatusPatcher + repo *provisioning.Repository + expectedErr string + }{ + { + name: "No finalizers", + repoFactory: nil, + finalizer: nil, + client: nil, + statusPatcher: nil, + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Finalizers: []string{}, + }, + }, + }, + { + name: "Finalizers deleted successfully", + repoFactory: func() repository.Factory { + f := repository.NewMockFactory(t) + + f. + On("Build", context.Background(), mock.Anything). + Once(). + Return(nil, nil) + + return f + }(), + finalizer: func() finalizerProcessor { + f := NewMockFinalizerProcessor(t) + + f. + On("process", context.Background(), nil, []string{ + repository.RemoveOrphanResourcesFinalizer, + }). + Once(). + Return(nil) + + return f + }(), + client: func() client.ProvisioningV0alpha1Interface { + repo := &mockRepoInterface{ + patchFunc: func(ctx context.Context, name string, pt types.PatchType, data []byte, opts metav1.PatchOptions, subresources ...string) (result *provisioning.Repository, err error) { + return &provisioning.Repository{}, nil + }, + } + c := &mockProvisioningV0alpha1Interface{ + repositoriesFunc: func(namespace string) client.RepositoryInterface { + return repo + }, + } + + return c + }(), + statusPatcher: nil, + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Finalizers: []string{ + repository.RemoveOrphanResourcesFinalizer, + }, + }, + }, + }, + { + name: "Error when building repository", + repoFactory: func() repository.Factory { + f := repository.NewMockFactory(t) + + f. + On("Build", context.Background(), mock.Anything). + Once(). + Return(nil, assert.AnError) + + return f + }(), + finalizer: nil, + client: nil, + statusPatcher: nil, + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Finalizers: []string{ + repository.RemoveOrphanResourcesFinalizer, + }, + }, + }, + expectedErr: "create repository from configuration: " + assert.AnError.Error(), + }, + { + name: "Error when processing finalizer", + repoFactory: func() repository.Factory { + f := repository.NewMockFactory(t) + + f. + On("Build", context.Background(), mock.Anything). + Once(). + Return(nil, nil) + + return f + }(), + finalizer: func() finalizerProcessor { + f := NewMockFinalizerProcessor(t) + + f. + On("process", context.Background(), nil, []string{ + repository.RemoveOrphanResourcesFinalizer, + }). + Once(). + Return(assert.AnError) + + return f + }(), + statusPatcher: func() StatusPatcher { + s := mocks.NewStatusPatcher(t) + + s. + On("Patch", context.Background(), mock.AnythingOfType("*v0alpha1.Repository"), mock.AnythingOfType("map[string]interface {}")). + Once(). + Return(nil) // Return nil error for the status patch + + return s + }(), + client: nil, + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Finalizers: []string{ + repository.RemoveOrphanResourcesFinalizer, + }, + }, + }, + expectedErr: "process finalizers: " + assert.AnError.Error(), + }, + { + name: "Error when patching finalizers", + repoFactory: func() repository.Factory { + f := repository.NewMockFactory(t) + + f. + On("Build", context.Background(), mock.Anything). + Once(). + Return(nil, nil) + + return f + }(), + finalizer: func() finalizerProcessor { + f := NewMockFinalizerProcessor(t) + + f. + On("process", context.Background(), nil, []string{ + repository.RemoveOrphanResourcesFinalizer, + }). + Once(). + Return(nil) + + return f + }(), + client: func() client.ProvisioningV0alpha1Interface { + repo := &mockRepoInterface{ + patchFunc: func(ctx context.Context, name string, pt types.PatchType, data []byte, opts metav1.PatchOptions, subresources ...string) (result *provisioning.Repository, err error) { + return &provisioning.Repository{}, assert.AnError + }, + } + c := &mockProvisioningV0alpha1Interface{ + repositoriesFunc: func(namespace string) client.RepositoryInterface { + return repo + }, + } + + return c + }(), + statusPatcher: nil, + repo: &provisioning.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Finalizers: []string{ + repository.RemoveOrphanResourcesFinalizer, + }, + }, + }, + expectedErr: "remove finalizers: " + assert.AnError.Error(), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + c := &RepositoryController{ + repoFactory: tc.repoFactory, + finalizer: tc.finalizer, + client: tc.client, + statusPatcher: tc.statusPatcher, + } + + err := c.handleDelete(context.Background(), tc.repo) + if tc.expectedErr != "" { + assert.Error(t, err) + assert.ErrorContains(t, err, tc.expectedErr) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/pkg/registry/apis/provisioning/jobs.go b/pkg/registry/apis/provisioning/jobs.go index 52a9cddcedf..4a344b60192 100644 --- a/pkg/registry/apis/provisioning/jobs.go +++ b/pkg/registry/apis/provisioning/jobs.go @@ -101,8 +101,18 @@ func (c *jobsConnector) Connect( responder.Error(err) return } + cfg := repo.Config() + if cfg.DeletionTimestamp != nil && !cfg.DeletionTimestamp.IsZero() { + responder.Error(apierrors.NewConflict( + provisioning.RepositoryResourceInfo.GroupResource(), + "cannot create jobs for a repository marked for deletion", + fmt.Errorf("cannot create jobs for a repository marked for deletion"), + )) + return + } + if idx > 0 { responder.Error(apierrors.NewBadRequest("can not post to a job UID")) return diff --git a/pkg/registry/apis/provisioning/jobs/driver.go b/pkg/registry/apis/provisioning/jobs/driver.go index 258a2af04b5..bad1df13759 100644 --- a/pkg/registry/apis/provisioning/jobs/driver.go +++ b/pkg/registry/apis/provisioning/jobs/driver.go @@ -271,6 +271,7 @@ func (d *jobDriver) processJobWithLeaseCheck(ctx context.Context, job *provision } func (d *jobDriver) processJob(ctx context.Context, job *provisioning.Job, recorder JobProgressRecorder) error { + logger := logging.FromContext(ctx) for _, worker := range d.workers { if !worker.IsSupported(ctx, *job) { continue @@ -281,6 +282,16 @@ func (d *jobDriver) processJob(ctx context.Context, job *provisioning.Job, recor return apifmt.Errorf("failed to get repository '%s': %w", job.Spec.Repository, err) } + r := repo.Config() + if r.DeletionTimestamp != nil && !r.DeletionTimestamp.IsZero() { + logger.Info("repository is marked for deletion, skipping processing job", + "name", r.Name, + "namespace", r.Namespace, + "deletionTimestamp", r.DeletionTimestamp, + ) + return nil + } + return worker.Process(ctx, repo, *job, recorder) } diff --git a/pkg/registry/apis/provisioning/resources/client.go b/pkg/registry/apis/provisioning/resources/client.go index d74d9d653ed..6a68a8add31 100644 --- a/pkg/registry/apis/provisioning/resources/client.go +++ b/pkg/registry/apis/provisioning/resources/client.go @@ -259,7 +259,7 @@ func (c *resourceClients) ForResource(ctx context.Context, gvr schema.GroupVersi Resource: gvr.Resource, }) if err != nil { - return nil, schema.GroupVersionKind{}, err + return nil, schema.GroupVersionKind{}, fmt.Errorf("getting preferred version for %s: %w", versionless.String(), err) } info, ok := c.byResource[gvr] @@ -270,7 +270,7 @@ func (c *resourceClients) ForResource(ctx context.Context, gvr schema.GroupVersi } else { gvk, err = discovery.GetKindForResource(gvr) if err != nil { - return nil, schema.GroupVersionKind{}, err + return nil, schema.GroupVersionKind{}, fmt.Errorf("getting kind for resource for %s: %w", gvr.String(), err) } } info = &clientInfo{ diff --git a/pkg/services/apiserver/client/discovery.go b/pkg/services/apiserver/client/discovery.go index db5fb26cccb..2eff7fa1470 100644 --- a/pkg/services/apiserver/client/discovery.go +++ b/pkg/services/apiserver/client/discovery.go @@ -70,7 +70,7 @@ func (d *DiscoveryClientImpl) GetKindForResource(gvr schema.GroupVersionResource func (d *DiscoveryClientImpl) GetPreferredVesion(gr schema.GroupResource) (schema.GroupVersionResource, schema.GroupVersionKind, error) { apiList, err := d.ServerPreferredResources() if err != nil { - return schema.GroupVersionResource{}, schema.GroupVersionKind{}, err + return schema.GroupVersionResource{}, schema.GroupVersionKind{}, fmt.Errorf("getting server's preferred resources: %w", err) } for _, apis := range apiList { if !strings.HasPrefix(apis.GroupVersion, gr.Group) { 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 3b31373c5ff..3e2e3a1644b 100644 --- a/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json +++ b/pkg/tests/apis/openapi_snapshots/provisioning.grafana.app-v0alpha1.json @@ -4218,6 +4218,10 @@ "webhook" ], "properties": { + "deleteError": { + "description": "Error information during repository deletion (if any)", + "type": "string" + }, "health": { "description": "This will get updated with the current health status (and updated periodically)", "default": {}, diff --git a/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts b/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts index aef543c403a..b43299a4d8a 100644 --- a/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts +++ b/public/app/api/clients/provisioning/v0alpha1/endpoints.gen.ts @@ -1289,6 +1289,8 @@ export type WebhookStatus = { url?: string; }; export type RepositoryStatus = { + /** Error information during repository deletion (if any) */ + deleteError?: string; /** This will get updated with the current health status (and updated periodically) */ health: HealthStatus; /** The generation of the spec last time reconciliation ran */