diff --git a/pkg/apiserver/registry/generic/strategy.go b/pkg/apiserver/registry/generic/strategy.go index 2628292fbee..6cb5dd918fd 100644 --- a/pkg/apiserver/registry/generic/strategy.go +++ b/pkg/apiserver/registry/generic/strategy.go @@ -4,6 +4,7 @@ import ( "context" "github.com/grafana/grafana/pkg/apimachinery/utils" + apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" @@ -41,15 +42,29 @@ func (g *genericStrategy) GetResetFields() map[fieldpath.APIVersion]*fieldpath.S return fields } -func (g *genericStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {} - -func (g *genericStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { - oldMeta, err := utils.MetaAccessor(old) +func (g *genericStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { + meta, err := utils.MetaAccessor(obj) if err != nil { return } + meta.SetGeneration(1) + objCopy := obj.DeepCopyObject() + err = runtime.SetZeroValue(objCopy) + if err != nil { + return + } + metaCopy, err := utils.MetaAccessor(objCopy) + if err != nil { + return + } + status, err := metaCopy.GetStatus() + if err == nil { + _ = meta.SetStatus(status) + } +} - status, err := oldMeta.GetStatus() +func (g *genericStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { + oldMeta, err := utils.MetaAccessor(old) if err != nil { return } @@ -59,7 +74,27 @@ func (g *genericStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime return } - _ = newMeta.SetStatus(status) + // update shouldn't change the status + status, err := oldMeta.GetStatus() + if err != nil { + _ = newMeta.SetStatus(nil) + } else { + _ = newMeta.SetStatus(status) + } + + spec, err := newMeta.GetSpec() + if err != nil { + return + } + + oldSpec, err := oldMeta.GetSpec() + if err != nil { + return + } + + if !apiequality.Semantic.DeepEqual(spec, oldSpec) { + newMeta.SetGeneration(oldMeta.GetGeneration() + 1) + } } func (g *genericStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { diff --git a/pkg/apiserver/registry/generic/strategy_test.go b/pkg/apiserver/registry/generic/strategy_test.go new file mode 100644 index 00000000000..969332305a2 --- /dev/null +++ b/pkg/apiserver/registry/generic/strategy_test.go @@ -0,0 +1,134 @@ +package generic_test + +import ( + "context" + "testing" + + "github.com/grafana/grafana/pkg/apiserver/registry/generic" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/apis/example" +) + +func TestPrepareForUpdate(t *testing.T) { + ctx := context.TODO() + gv := schema.GroupVersion{Group: "test", Version: "v1"} + strategy := generic.NewStrategy(runtime.NewScheme(), gv) + + oldObj := &example.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Generation: 1, + }, + Spec: example.PodSpec{ + NodeSelector: map[string]string{"foo": "bar"}, + }, + Status: example.PodStatus{ + Phase: example.PodPhase("Running"), + }, + } + + testCases := []struct { + name string + newObj *example.Pod + oldObj *example.Pod + expectedGen int64 + expectedObj *example.Pod + }{ + { + name: "ignore status updates", + newObj: &example.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Generation: 1, + }, + Spec: example.PodSpec{ + NodeSelector: map[string]string{"foo": "bar"}, + }, + Status: example.PodStatus{ + Phase: example.PodPhase("Stopped"), + }, + }, + oldObj: oldObj.DeepCopy(), + expectedGen: 2, + expectedObj: &example.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Generation: 1, + }, + Spec: example.PodSpec{ + NodeSelector: map[string]string{"foo": "bar"}, + }, + Status: example.PodStatus{ + Phase: example.PodPhase("Running"), + }, + }, + }, + { + name: "increment generation if spec changes", + newObj: &example.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Generation: 1, + }, + Spec: example.PodSpec{ + NodeSelector: map[string]string{"foo": "baz"}, + }, + Status: example.PodStatus{ + Phase: example.PodPhase("Running"), + }, + }, + oldObj: oldObj.DeepCopy(), + expectedGen: 2, + expectedObj: &example.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Generation: 2, + }, + Spec: example.PodSpec{ + NodeSelector: map[string]string{"foo": "baz"}, + }, + Status: example.PodStatus{ + Phase: example.PodPhase("Running"), + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + strategy.PrepareForUpdate(ctx, tc.newObj, tc.oldObj) + require.Equal(t, tc.expectedObj, tc.newObj) + }) + } +} + +func TestPrepareForCreate(t *testing.T) { + ctx := context.TODO() + gv := schema.GroupVersion{Group: "test", Version: "v1"} + strategy := generic.NewStrategy(runtime.NewScheme(), gv) + + obj := &example.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Spec: example.PodSpec{ + NodeSelector: map[string]string{"foo": "bar"}, + }, + Status: example.PodStatus{ + Phase: example.PodPhase("Running"), + }, + } + + strategy.PrepareForCreate(ctx, obj) + require.Equal(t, int64(1), obj.Generation) + require.Equal(t, example.PodStatus{}, obj.Status) +} diff --git a/pkg/tests/apis/helper.go b/pkg/tests/apis/helper.go index dd4c8e8c03c..0e2685e432b 100644 --- a/pkg/tests/apis/helper.go +++ b/pkg/tests/apis/helper.go @@ -216,6 +216,9 @@ func (c *K8sResourceClient) sanitizeObject(v *unstructured.Unstructured, replace meta, ok := copy["metadata"].(map[string]any) require.True(c.t, ok) + // remove generation + delete(meta, "generation") + replaceMeta = append(replaceMeta, "creationTimestamp", "resourceVersion", "uid") for _, key := range replaceMeta { old, ok := meta[key] @@ -403,12 +406,18 @@ func DoRequest[T any](c *K8sTestHelper, params RequestParams, result *T) K8sResp // Read local JSON or YAML file into a resource func (c *K8sTestHelper) LoadYAMLOrJSONFile(fpath string) *unstructured.Unstructured { c.t.Helper() + return c.LoadYAMLOrJSON(string(c.LoadFile(fpath))) +} + +// Read local file into a byte slice. Does not need to be a resource. +func (c *K8sTestHelper) LoadFile(fpath string) []byte { + c.t.Helper() //nolint:gosec raw, err := os.ReadFile(fpath) require.NoError(c.t, err) require.NotEmpty(c.t, raw) - return c.LoadYAMLOrJSON(string(raw)) + return raw } // Read local JSON or YAML file into a resource