terraform: fix security rule reconciliation on Azure

.github/workflows/e2e-upgrade.yml

@@ -40,7 +40,7 @@ on:
         type: string
         required: false
       toMicroservices:
-        description: Microservice version to target for the upgrade, empty for upgrade target's default version.
+        description: Microservice version to target for the upgrade, empty for upgrade target's default version. When simulating a patch upgrade, you must set this value because the default will not be the simulated version.
         type: string
         required: false
       simulatedTargetVersion:
@@ -404,15 +404,9 @@ jobs:
           echo "K8s target: $KUBERNETES"
           echo "Microservice target: $MICROSERVICES"
 
-          if [[ -n ${MICROSERVICES} ]]; then
-            MICROSERVICES_FLAG="--target-microservices=$MICROSERVICES"
-          fi
-          if [[ -n ${KUBERNETES} ]]; then
-            KUBERNETES_FLAG="--target-kubernetes=$KUBERNETES"
-          fi
-
           sudo sh -c 'echo "127.0.0.1 license.confidential.cloud" >> /etc/hosts'
-          bazel run --test_timeout=14400 //e2e/internal/upgrade:upgrade_test -- --want-worker "$WORKERNODES" --want-control "$CONTROLNODES" --target-image "$IMAGE" "$KUBERNETES_FLAG" "$MICROSERVICES_FLAG"
+          CLI=$(realpath ./build/constellation)
+          bazel run --test_timeout=14400 //e2e/internal/upgrade:upgrade_test -- --want-worker "$WORKERNODES" --want-control "$CONTROLNODES" --target-image "$IMAGE" --target-kubernetes "$KUBERNETES" --target-microservices "$MICROSERVICES" --cli "$CLI"
 
       - name: Remove Terraform plugin cache
         if: always()

dev-docs/workflows/e2e-github-config.md → dev-docs/workflows/e2e-upgrade-config.md

@@ -1,6 +1,9 @@
-## [E2E upgrade test]((https://github.com/edgelesssys/constellation/actions/workflows/e2e-upgrade.yml)
+## [E2E upgrade test](https://github.com/edgelesssys/constellation/actions/workflows/e2e-upgrade.yml)
+
 Make sure to set the correct parameters to avoid late failures:
+
 - it's easiest to use the latest CLI version, because then you can omit all other fields. This works because the devbuild is tagged with the next release version and hence is compatible.
 - if using an older CLI version:
   - the last field about simulating a patch-upgrade must have a minor version that is smaller by one compared to the next release
   - the image version must match the patch field version
+  - the service version must match the patch field version

e2e/internal/upgrade/upgrade.go

@@ -297,10 +297,10 @@ func getCLIPath(cliPathFlag string) (string, error) {
 	pathCLI := os.Getenv("PATH_CLI")
 	var relCLIPath string
 	switch {
-	case pathCLI != "":
-		relCLIPath = pathCLI
 	case cliPathFlag != "":
 		relCLIPath = cliPathFlag
+	case pathCLI != "":
+		relCLIPath = pathCLI
 	default:
 		return "", errors.New("neither 'PATH_CLI' nor 'cli' flag set")
 	}

e2e/internal/upgrade/upgrade_test.go

@@ -81,7 +81,7 @@ func TestUpgrade(t *testing.T) {
 	log.Println(string(data))
 
 	log.Println("Checking upgrade.")
-	runUpgradeCheck(require, cli, *targetKubernetes)
+	runUpgradeCheck(require, cli, *targetKubernetes, *targetMicroservices)
 
 	log.Println("Triggering upgrade.")
 	runUpgradeApply(require, cli)
@@ -170,7 +170,7 @@ func testNodesEventuallyAvailable(t *testing.T, k *kubernetes.Clientset, wantCon
 
 // runUpgradeCheck executes 'upgrade check' and does basic checks on the output.
 // We can not check images upgrades because we might use unpublished images. CLI uses public CDN to check for available images.
-func runUpgradeCheck(require *require.Assertions, cli, targetKubernetes string) {
+func runUpgradeCheck(require *require.Assertions, cli, targetKubernetes string, targetMicroservices string) {
 	cmd := exec.CommandContext(context.Background(), cli, "upgrade", "check", "--debug")
 	stdout, stderr, err := runCommandWithSeparateOutputs(cmd)
 	require.NoError(err, "Stdout: %s\nStderr: %s", string(stdout), string(stderr))
@@ -186,9 +186,12 @@ func runUpgradeCheck(require *require.Assertions, cli, targetKubernetes string)
 		log.Printf("false. targetKubernetes: %s\n", targetKubernetes)
 		require.Contains(string(stdout), targetKubernetes, fmt.Sprintf("Expected Kubernetes version %s in output.", targetKubernetes))
 	}
+	if targetMicroservices == "" {
+		targetMicroservices = constants.BinaryVersion().String() // assume that the CLI is not a simulated patch upgrade, such that the version is the same as the CLI version.
+	}
 
 	require.Contains(string(stdout), "Services:")
-	require.Contains(string(stdout), fmt.Sprintf("--> %s", constants.BinaryVersion().String()))
+	require.Contains(string(stdout), fmt.Sprintf("--> %s", targetMicroservices))
 
 	log.Println(string(stdout))
 }

e2e/provider-upgrade/upgrade_test.go

@@ -25,7 +25,7 @@ import (
 var (
 	targetImage         = flag.String("target-image", "", "Image (shortversion) to upgrade to.")
 	targetKubernetes    = flag.String("target-kubernetes", "", "Kubernetes version (MAJOR.MINOR.PATCH) to upgrade to. Defaults to default version of target CLI.")
-	targetMicroservices = flag.String("target-microservices", "", "Microservice version (MAJOR.MINOR.PATCH) to upgrade to. Defaults to default version of target CLI.")
+	targetMicroservices = flag.String("target-microservices", "", "Microservice version (MAJOR.MINOR.PATCH) to upgrade to. Defaults to the pre version of the next release. You must set this value when simulating a patch upgrade.")
 	// When executing the test as a bazel target the CLI path is supplied through an env variable that bazel sets.
 	// When executing via `go test` extra care should be taken that the supplied CLI is built on the same commit as this test.
 	// When executing the test as a bazel target the workspace path is supplied through an env variable that bazel sets.

terraform/infrastructure/azure/main.tf

@@ -227,36 +227,15 @@ resource "azurerm_network_security_group" "security_group" {
   location            = var.location
   resource_group_name = var.resource_group
   tags                = local.tags
-
-  dynamic "security_rule" {
-    # we keep this rule for one last release since the azurerm provider does not 
-    # support moving security rules that are inlined (like this) to the external resource one.
-    # Even worse, just defining the azurerm_network_security_group without the 
-    # "security_rule" block will NOT remove all the rules but do nothing.
-    # TODO(@3u13r): remove the "security_rule" block in the next release after this code has landed.
-    # So either after 2.19 or after 2.18.X if cherry-picked release.
-    for_each = [{ name = "konnectivity", priority = 1000, port = 8132 }]
-    content {
-      name                       = security_rule.value.name
-      priority                   = security_rule.value.priority
-      direction                  = "Inbound"
-      access                     = "Allow"
-      protocol                   = "Tcp"
-      source_port_range          = "*"
-      destination_port_range     = security_rule.value.port
-      source_address_prefix      = "*"
-      destination_address_prefix = "*"
-    }
-  }
 }
 
 resource "azurerm_network_security_rule" "nsg_rule" {
   for_each = {
     for o in local.ports : o.name => o
   }
-
-  name                        = each.value.name
-  priority                    = each.value.priority
+  # TODO(elchead): v2.20.0: remove name suffix and priority offset. Might need to add create_before_destroy to the NSG rule.
+  name                        = "${each.value.name}-new"
+  priority                    = each.value.priority + 10 # offset to not overlap with old rules
   direction                   = "Inbound"
   access                      = "Allow"
   protocol                    = "Tcp"