Merge branch 'main' into fix/classic-validator-early-returns-on-different-scopes

Author: d0weinberger

cmd/monaco/integrationtest/utils/monaco/cmd.go

@@ -47,7 +47,6 @@ func Run(t *testing.T, fs afero.Fs, command string) error {
 	if !strings.HasPrefix(c, prefix) {
 		return fmt.Errorf("command must start with '%s'", prefix)
 	}
-	t.Logf("Running command: %s", command)
 	c = strings.TrimPrefix(c, prefix)
 
 	args := strings.Split(c, " ")
@@ -58,5 +57,10 @@ func Run(t *testing.T, fs afero.Fs, command string) error {
 	// explicit cancel for each monaco run invocation
 	ctx, cancel := context.WithCancel(t.Context())
 	defer cancel()
-	return runner.RunCmd(ctx, cmd)
+
+	t.Logf("Running command: %s", command)
+	err := runner.RunCmd(ctx, cmd)
+	t.Logf("Finished command: %s", command)
+
+	return err
 }

cmd/monaco/integrationtest/v2/integration_test_utils.go

@@ -137,6 +137,7 @@ func runIntegration(t *testing.T, opts testOptions, testFunc TestFunc) {
 
 	if !opts.skipCleanup {
 		defer func() {
+			t.Log("Starting cleanup")
 			integrationtest.CleanupIntegrationTest(t, opts.fs, opts.manifestPath, opts.specificEnvironment, suffix)
 		}()
 	}

cmd/monaco/integrationtest/v2/settings_integration_test.go

@@ -398,7 +398,7 @@ func TestOrdered_InsertAtFrontAndBackWorks(t *testing.T) {
 	const manifestFile = configFolder + "/manifest.yaml"
 
 	const specificEnvironment = "platform"
-	const project = "both-back-and-front-are-set"
+	const project = "both-back-and-front-are-set-with-initial"
 	const schema = "builtin:url-based-sampling"
 
 	RunIntegrationWithCleanup(t, configFolder, manifestFile, specificEnvironment, "InsertAtBackWorks", func(fs afero.Fs, tc TestContext) {
@@ -417,6 +417,48 @@ func TestOrdered_InsertAtFrontAndBackWorks(t *testing.T) {
 			Filter:       filterObjectsForScope(pgiMeId),
 		})
 
+		assert.Equal(t, 3, len(list), "Exactly three configs should be deployed")
+
+		// Verify that last is actually the first object
+		first := settingsExternalIdForTest(t, coordinate.Coordinate{Project: project, Type: schema, ConfigId: "first"}, tc)
+		assert.Equal(t, 0, findPositionWithExternalId(t, list, first))
+
+		// Verify that last is actually the last object
+		last := settingsExternalIdForTest(t, coordinate.Coordinate{Project: project, Type: schema, ConfigId: "last"}, tc)
+		assert.Equal(t, len(list)-1, findPositionWithExternalId(t, list, last))
+	})
+}
+
+func TestOrdered_InsertAtFrontAndBackWorksDeployTwice(t *testing.T) {
+	const configFolder = "test-resources/settings-ordered/insert-position"
+
+	const manifestFile = configFolder + "/manifest.yaml"
+
+	const specificEnvironment = "platform"
+	const project = "both-back-and-front-are-set-deploy-twice"
+	const schema = "builtin:url-based-sampling"
+
+	RunIntegrationWithCleanup(t, configFolder, manifestFile, specificEnvironment, "InsertAtBackWorks", func(fs afero.Fs, tc TestContext) {
+		pgiMeId := randomMeID("PROCESS_GROUP_INSTANCE")
+		setTestEnvVar(t, "MONACO_TARGET_ENTITY_SCOPE", pgiMeId, tc.suffix)
+		t.Log("Monitored entity ID for testing ('MONACO_TARGET_ENTITY_SCOPE') =", pgiMeId)
+
+		// first
+		err := monaco.Run(t, fs, fmt.Sprintf("monaco deploy %s --project %s --verbose", manifestFile, project))
+		require.NoError(t, err)
+		// second
+		err = monaco.Run(t, fs, fmt.Sprintf("monaco deploy %s --project %s --verbose", manifestFile, project))
+		require.NoError(t, err)
+
+		integrationtest.AssertAllConfigsAvailability(t, fs, manifestFile, []string{project}, specificEnvironment, true)
+
+		sClient := createSettingsClientFromManifest(t, fs, manifestFile, "platform")
+
+		list, err := sClient.List(t.Context(), schema, dtclient.ListSettingsOptions{
+			DiscardValue: true,
+			Filter:       filterObjectsForScope(pgiMeId),
+		})
+
 		assert.Equal(t, 2, len(list), "Exactly two configs should be deployed")
 
 		// Verify that last is actually the first object

cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/both-back-and-front-are-set/config.yaml cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/both-back-and-front-are-set-deploy-twice/config.yaml

@@ -1,4 +1,4 @@
-# Test if setting `insertAfter` to `BACK` works as expected.
+# Test if setting `insertAfter` to `BACK` works as expected when deploying the configs twice
 #
 # First config, `first`, is deployed and will be added to the front
 # Second config, `last`, will be added to the back.
@@ -15,6 +15,7 @@ configs:
       insertAfter: FRONT
   config:
     template: url-sampling.json
+    name: first
 
 - id: last
   type:
@@ -26,3 +27,4 @@ configs:
       insertAfter: BACK
   config:
     template: url-sampling.json
+    name: last

cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/both-back-and-front-are-set/url-sampling.json cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/both-back-and-front-are-set-deploy-twice/url-sampling.json

@@ -1,9 +1,9 @@
-{
-    "enabled": true,
-    "ignore": false,
-    "factor": "6",
-    "path": "test",
-    "pathComparisonType": "EQUALS",
-    "queryParameters": [],
-    "httpMethodAny": true
-}
+{
+    "enabled": true,
+    "ignore": false,
+    "factor": "6",
+    "path": "{{ .name }}",
+    "pathComparisonType": "EQUALS",
+    "queryParameters": [],
+    "httpMethodAny": true
+}

cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/both-back-and-front-are-set-with-initial/config.yaml

@@ -0,0 +1,52 @@
+# Test if setting `insertAfter` to `BACK` works as expected with an initial config as 'separator'.
+# Without the separator, the configs are deployed in parallel and might run into a race condition on the remote server leading to an unpredictable ordering.
+#
+# First, `initial` is deployed. Then `first` and `last` are deployed in parallel.
+# `first` should be on the first position, `last` on the last position.
+
+configs:
+- id: initial
+  type:
+    settings:
+      schema: builtin:url-based-sampling
+      scope:
+        type: environment
+        name: MONACO_TARGET_ENTITY_SCOPE
+      insertAfter: FRONT
+  config:
+    template: url-sampling.json
+    name: initial
+
+- id: first
+  type:
+    settings:
+      schema: builtin:url-based-sampling
+      scope:
+        type: environment
+        name: MONACO_TARGET_ENTITY_SCOPE
+      insertAfter: FRONT
+  config:
+    template: url-sampling.json
+    name: first
+    parameters:
+      _phantom:
+        type: reference
+        property: id
+        configId: initial
+
+- id: last
+  type:
+    settings:
+      schema: builtin:url-based-sampling
+      scope:
+        type: environment
+        name: MONACO_TARGET_ENTITY_SCOPE
+      insertAfter: BACK
+  config:
+    template: url-sampling.json
+    name: last
+    parameters:
+      _phantom:
+        type: reference
+        property: id
+        configId: initial

cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/both-back-and-front-are-set-with-initial/url-sampling.json

@@ -0,0 +1,9 @@
+{
+    "enabled": true,
+    "ignore": false,
+    "factor": "6",
+    "path": "{{ .name }}",
+    "pathComparisonType": "EQUALS",
+    "queryParameters": [],
+    "httpMethodAny": true
+}

cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/insert-after-not-set/config.yaml

@@ -14,6 +14,7 @@ configs:
         name: MONACO_TARGET_ENTITY_SCOPE
   config:
     template: url-sampling.json
+    name: first
 
 - id: second
   type:
@@ -25,6 +26,7 @@ configs:
 
   config:
     template: url-sampling.json
+    name: second
     parameters:
       phantom:
         type: reference

cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/insert-after-not-set/url-sampling.json

@@ -2,7 +2,7 @@
     "enabled": true,
     "ignore": false,
     "factor": "6",
-    "path": "test",
+    "path": "{{ .name }}",
     "pathComparisonType": "EQUALS",
     "queryParameters": [],
     "httpMethodAny": true

cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/insert-after-set-to-back/config.yaml

@@ -7,6 +7,7 @@ configs:
 - id: first
   config:
     template: url-sampling.json
+    name: first
   type:
     settings:
       schema: builtin:url-based-sampling
@@ -16,6 +17,7 @@ configs:
 - id: second
   config:
     template: url-sampling.json
+    name: second
     parameters:
       phantom:
         type: reference

cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/insert-after-set-to-back/url-sampling.json

@@ -2,7 +2,7 @@
     "enabled": true,
     "ignore": false,
     "factor": "6",
-    "path": "test",
+    "path": "{{ .name }}",
     "pathComparisonType": "EQUALS",
     "queryParameters": [],
     "httpMethodAny": true

cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/insert-after-set-to-front/config.yaml

@@ -13,6 +13,7 @@ configs:
         name: MONACO_TARGET_ENTITY_SCOPE
   config:
     template: url-sampling.json
+    name: first
 
 - id: second
   type:
@@ -24,6 +25,7 @@ configs:
       insertAfter: FRONT
   config:
     template: url-sampling.json
+    name: second
     parameters:
       phantom-to-first:
         type: reference
@@ -43,5 +45,6 @@ configs:
         property: id
   config:
     template: url-sampling.json
+    name: third
 
 

cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/insert-after-set-to-front/url-sampling.json

@@ -2,7 +2,7 @@
     "enabled": true,
     "ignore": false,
     "factor": "6",
-    "path": "test",
+    "path": "{{ .name }}",
     "pathComparisonType": "EQUALS",
     "queryParameters": [],
     "httpMethodAny": true

cmd/monaco/integrationtest/v2/test-resources/settings-ordered/insert-position/manifest.yaml

@@ -1,7 +1,8 @@
 manifestVersion: 1.0
 
 projects:
-- name: both-back-and-front-are-set
+- name: both-back-and-front-are-set-with-initial
+- name: both-back-and-front-are-set-deploy-twice
 - name: insert-after-not-set
 - name: insert-after-set-to-back
 - name: insert-after-set-to-front

pkg/client/dtclient/settings_client.go

@@ -627,6 +627,11 @@ func (d *SettingsClient) Upsert(ctx context.Context, obj SettingsObject, upsertO
 	}
 
 	log.WithCtxFields(ctx).Debug("Created/Updated object %s (%s) with externalId %s", obj.Coordinate.ConfigId, obj.SchemaId, externalID)
+	insertAfterForLogging := "<nil>"
+	if upsertOptions.InsertAfter != nil {
+		insertAfterForLogging = *upsertOptions.InsertAfter
+	}
+	log.WithCtxFields(ctx).Debug("Created/Updated object %s (schemaID: %s, Scope: %s, insertAfter: %s) with externalId %s", obj.Coordinate.ConfigId, obj.SchemaId, obj.Scope, insertAfterForLogging, externalID)
 	return entity, nil
 }