cleanup

pkg/registry/apis/datasource/legacy_store.go

@@ -147,6 +147,7 @@ func (s *legacyStorage) Update(ctx context.Context, name string, objInfo rest.Up
 			}
 			secureChanges[k] = v
 			dualwrite.SetUpdatedSecureValues(ctx, ds.Secure)
+			continue
 		}
 
 		// The legacy store must use fixed names generated by the internal system

pkg/storage/legacysql/dualwrite/dualwriter.go

@@ -420,6 +420,14 @@ func (d *dualWriter) Update(ctx context.Context, name string, objInfo rest.Updat
 		}
 	}
 
+	// Propagate secure values from the update request to the unified storage update.
+	if secure := getUpdatedSecureValues(ctx); secure != nil {
+		wrapped, ok := unifiedInfo.(*wrappedUpdateInfo)
+		if ok {
+			wrapped.updatedSecureValues = secure
+		}
+	}
+
 	if d.readUnified {
 		return d.unified.Update(ctx, name, unifiedInfo, createValidation, updateValidation, unifiedForceCreate, options)
 	} else if d.errorIsOK {

pkg/tests/apis/datasource/testdata_test.go

@@ -5,6 +5,8 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"maps"
+	"slices"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -13,9 +15,11 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
-	"github.com/grafana/grafana/pkg/components/simplejson"
+	"github.com/grafana/grafana/pkg/apimachinery/utils"
+	grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
 	"github.com/grafana/grafana/pkg/services/datasources"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/tests/apis"
 	"github.com/grafana/grafana/pkg/tests/testinfra"
 	"github.com/grafana/grafana/pkg/tests/testsuite"
@@ -29,111 +33,177 @@ func TestMain(m *testing.M) {
 func TestIntegrationTestDatasource(t *testing.T) {
 	testutil.SkipIntegrationTestInShortMode(t)
 
-	helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{
-		AppModeProduction: false, // dev mode required for datasource connections
-		DisableAnonymous:  true,
-		EnableFeatureToggles: []string{
-			featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, // Required to start the example service
-		},
-	})
+	for _, mode := range []grafanarest.DualWriterMode{
+		// grafanarest.Mode0, // Legacy only
+		// grafanarest.Mode2, // write both, read legacy
+		// grafanarest.Mode3, // write both, read unified
+		grafanarest.Mode5, // Unified only
+	} {
+		t.Run(fmt.Sprintf("testdata (mode:%d)", mode), func(t *testing.T) {
+			ctx := context.Background()
+			helper := apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{
+				DisableAnonymous: true,
+				EnableFeatureToggles: []string{
+					featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, // Required to start the datasource api servers
+					featuremgmt.FlagQueryServiceWithConnections,          // enables CRUD endpoints
+				},
+				UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
+					"datasources.testdata.datasource.grafana.app": {
+						DualWriterMode: mode,
+					},
+				},
+			})
 
-	// Create a single datasource
-	ds := helper.CreateDS(&datasources.AddDataSourceCommand{
-		Name:  "test",
-		Type:  datasources.DS_TESTDATA,
-		UID:   "test",
-		OrgID: int64(1),
+			client := helper.Org1.Admin.ResourceClient(t, schema.GroupVersionResource{
+				Group:    "testdata.datasource.grafana.app",
+				Version:  "v0alpha1",
+				Resource: "datasources",
+			}).Namespace("default")
 
-		// These settings are not actually used, but testing that they get saved
-		Database: "testdb",
-		URL:      "http://fake.url",
-		Access:   datasources.DS_ACCESS_PROXY,
-		User:     "example",
-		ReadOnly: true,
-		JsonData: simplejson.NewFromAny(map[string]any{
-			"hello": "world",
-		}),
-		SecureJsonData: map[string]string{
-			"aaa": "AAA",
-			"bbb": "BBB",
-		},
-	})
-	require.Equal(t, "test", ds.UID)
+			// 1. CREATE
+			out, err := client.Create(ctx, &unstructured.Unstructured{
+				Object: map[string]any{
+					"apiVersion": "testdata.datasource.grafana.app/v0alpha1",
+					"kind":       "DataSource",
+					"metadata": map[string]any{
+						"name": "test",
+					},
+					"spec": map[string]any{
+						"title": "test",
+					},
+					"secure": map[string]any{
+						"aaa": map[string]any{
+							"create": "AAA",
+						},
+						"bbb": map[string]any{
+							"create": "BBB",
+						},
+					},
+				},
+			}, metav1.CreateOptions{})
+			require.NoError(t, err)
+			require.Equal(t, "test", out.GetName())
 
-	t.Run("Admin configs", func(t *testing.T) {
-		client := helper.Org1.Admin.ResourceClient(t, schema.GroupVersionResource{
-			Group:    "testdata.datasource.grafana.app",
-			Version:  "v0alpha1",
-			Resource: "datasources",
-		}).Namespace("default")
-		ctx := context.Background()
+			obj, err := utils.MetaAccessor(out)
+			require.NoError(t, err)
 
-		list, err := client.List(ctx, metav1.ListOptions{})
-		require.NoError(t, err)
-		require.Len(t, list.Items, 1, "expected a single connection")
-		require.Equal(t, "test", list.Items[0].GetName(), "with the test uid")
+			secure, err := obj.GetSecureValues()
+			require.NoError(t, err)
 
-		spec, _, _ := unstructured.NestedMap(list.Items[0].Object, "spec")
-		jj, _ := json.MarshalIndent(spec, "", "  ")
-		fmt.Printf("%s\n", string(jj))
-		require.JSONEq(t, `{
-			"access": "proxy",
-			"database": "testdb",
-			"isDefault": true,
-			"jsonData": {
-				"hello": "world"
-			},
-			"readOnly": true,
-			"title": "test",
-			"url": "http://fake.url",
-			"user": "example"
-		}`, string(jj))
-	})
+			keys := slices.Collect(maps.Keys(secure))
+			require.ElementsMatch(t, []string{"aaa", "bbb"}, keys)
 
-	t.Run("Call subresources", func(t *testing.T) {
-		client := helper.Org1.Admin.ResourceClient(t, schema.GroupVersionResource{
-			Group:    "testdata.datasource.grafana.app",
-			Version:  "v0alpha1",
-			Resource: "datasources",
-		}).Namespace("default")
-		ctx := context.Background()
+			// 2. UPDATE
+			out, err = client.Update(ctx, &unstructured.Unstructured{
+				Object: map[string]any{
+					"apiVersion": "testdata.datasource.grafana.app/v0alpha1",
+					"metadata": map[string]any{
+						"name": "test",
+					},
+					"spec": map[string]any{
+						"title":     "test",
+						"database":  "testdb",
+						"url":       "http://fake.url",
+						"access":    datasources.DS_ACCESS_PROXY,
+						"user":      "example",
+						"isDefault": true,
+						"readOnly":  true,
+						"jsonData": map[string]any{
+							"hello": "world",
+						},
+					},
+					"secure": map[string]any{
+						"aaa": map[string]any{
+							"remove": true, // remove the first secure value
+						},
+						"ccc": map[string]any{
+							"create": "CCC", // add a third value
+						},
+					},
+				},
+			}, metav1.UpdateOptions{})
+			require.NoError(t, err)
+			require.Equal(t, "test", out.GetName())
 
-		list, err := client.List(ctx, metav1.ListOptions{})
-		require.NoError(t, err)
-		require.Len(t, list.Items, 1, "expected a single connection")
-		require.Equal(t, "test", list.Items[0].GetName(), "with the test uid")
+			obj, err = utils.MetaAccessor(out)
+			require.NoError(t, err)
 
-		_, err = client.Get(ctx, "test", metav1.GetOptions{}, "health")
-		// endpoint is disabled currently because it has not been
-		// sufficiently tested.
-		// for more info see pkg/registry/apis/datasource/sub_health.go
-		require.Error(t, err)
-		var statusErr *apierrors.StatusError
-		require.True(t, errors.As(err, &statusErr))
-		require.Equal(t, int32(501), statusErr.ErrStatus.Code)
-		// require.NoError(t, err)
-		// body, err := rsp.MarshalJSON()
-		// require.NoError(t, err)
-		// //fmt.Printf("GOT: %v\n", string(body))
-		// require.JSONEq(t, `{
-		// 	"apiVersion": "testdata.datasource.grafana.app/v0alpha1",
-		// 	"code": 1,
-		// 	"kind": "HealthCheckResult",
-		// 	"message": "Data source is working",
-		// 	"status": "OK"
-		//   }
-		// `, string(body))
+			secure, err = obj.GetSecureValues()
+			require.NoError(t, err)
+
+			keys = slices.Collect(maps.Keys(secure))
+			require.ElementsMatch(t, []string{"bbb", "ccc"}, keys)
+
+			// 3. LIST
+			list, err := client.List(ctx, metav1.ListOptions{})
+			require.NoError(t, err)
+			require.Len(t, list.Items, 1, "expected a single datasource")
+			require.Equal(t, "test", list.Items[0].GetName(), "with the test uid")
+
+			spec, _, _ := unstructured.NestedMap(list.Items[0].Object, "spec")
+			jj, _ := json.MarshalIndent(spec, "", "  ")
+			// fmt.Printf("%s\n", string(jj))
+			require.JSONEq(t, `{
+					"access": "proxy",
+					"database": "testdb",
+					"isDefault": true,
+					"jsonData": {
+						"hello": "world"
+					},
+					"readOnly": true,
+					"title": "test",
+					"url": "http://fake.url",
+					"user": "example"
+				}`, string(jj))
+
+			// 4. Call functions that use the stored configs
+			t.Run("subresources", func(t *testing.T) {
+				client := helper.Org1.Admin.ResourceClient(t, schema.GroupVersionResource{
+					Group:    "testdata.datasource.grafana.app",
+					Version:  "v0alpha1",
+					Resource: "datasources",
+				}).Namespace("default")
+				ctx := context.Background()
+
+				list, err := client.List(ctx, metav1.ListOptions{})
+				require.NoError(t, err)
+				require.Len(t, list.Items, 1, "expected a single connection")
+				require.Equal(t, "test", list.Items[0].GetName(), "with the test uid")
+
+				_, err = client.Get(ctx, "test", metav1.GetOptions{}, "health")
+				// endpoint is disabled currently because it has not been
+				// sufficiently tested.
+				// for more info see pkg/registry/apis/datasource/sub_health.go
+				require.Error(t, err)
+				var statusErr *apierrors.StatusError
+				require.True(t, errors.As(err, &statusErr))
+				require.Equal(t, int32(501), statusErr.ErrStatus.Code)
+				// require.NoError(t, err)
+				// body, err := rsp.MarshalJSON()
+				// require.NoError(t, err)
+				// //fmt.Printf("GOT: %v\n", string(body))
+				// require.JSONEq(t, `{
+				// 	"apiVersion": "testdata.datasource.grafana.app/v0alpha1",
+				// 	"code": 1,
+				// 	"kind": "HealthCheckResult",
+				// 	"message": "Data source is working",
+				// 	"status": "OK"
+				//   }
+				// `, string(body))
+
+				// Test connecting to non-JSON marshaled data
+				raw := apis.DoRequest[any](helper, apis.RequestParams{
+					User:   helper.Org1.Admin,
+					Method: "GET",
+					Path:   "/apis/testdata.datasource.grafana.app/v0alpha1/namespaces/default/datasources/test/resource",
+				}, nil)
+				// endpoint is disabled currently because it has not been
+				// sufficiently tested.
+				// for more info see pkg/registry/apis/datasource/sub_resource.go
+				require.Equal(t, int32(501), raw.Status.Code)
+				// require.Equal(t, `Hello world from test datasource!`, string(raw.Body))
+			})
+		})
+	}
 
-		// Test connecting to non-JSON marshaled data
-		raw := apis.DoRequest[any](helper, apis.RequestParams{
-			User:   helper.Org1.Admin,
-			Method: "GET",
-			Path:   "/apis/testdata.datasource.grafana.app/v0alpha1/namespaces/default/datasources/test/resource",
-		}, nil)
-		// endpoint is disabled currently because it has not been
-		// sufficiently tested.
-		// for more info see pkg/registry/apis/datasource/sub_resource.go
-		require.Equal(t, int32(501), raw.Status.Code)
-		// require.Equal(t, `Hello world from test datasource!`, string(raw.Body))
-	})
 }