Load the app/SP from the environment instead of automatically creating it (#3498)

* use multierror here, so it's more obvious if we're missing multiple keys

* Ignore the written out clusterapp.env

* move create/delete into separate commands, which write out a clusterapp.env file

* delete the app in the e2e.sh file

* update the docs

.gitignore

@@ -30,7 +30,6 @@ gomock_reflect_*
 /e2e-report.xml
 /deploy/config.yaml
 **/*.swp
-/portal/v1/node_modules/
 /portal/v2/node_modules/
 portal/v2/.vscode/
 .idea*
@@ -43,3 +42,4 @@ megalinter-reports/
 /jq
 /portalauth
 .kiota.log
+/clusterapp.env

.pipelines/e2e.yml

@@ -51,6 +51,7 @@ jobs:
 
           export CI=true
           . ./hack/e2e/run-rp-and-e2e.sh
+          get_cluster_sp
           deploy_e2e_db
         displayName: Setup (Azure)
 

docs/deploy-development-rp.md

@@ -112,6 +112,10 @@
    OR use the create utility:
 
    ```bash
+   # Create the application to run the cluster as and load it
+   CLUSTER=<cluster-name> go run ./hack/cluster createapp
+   source clusterapp.env
+   # Create the cluster
    CLUSTER=<cluster-name> go run ./hack/cluster create
    ```
 
@@ -119,6 +123,7 @@
 
    ```bash
    CLUSTER=<cluster-name> go run ./hack/cluster delete
+   CLUSTER=<cluster-name> go run ./hack/cluster deleteapp
    ```
 
    By default, a public cluster will be created. In order to create a private cluster, set the `PRIVATE_CLUSTER` environment variable to `true` prior to creation. Internet access from the cluster can also be restricted by setting the `NO_INTERNET` environment variable to `true`.

hack/cluster/cluster.go

@@ -25,7 +25,7 @@ const (
 
 func run(ctx context.Context, log *logrus.Entry) error {
 	if len(os.Args) != 2 {
-		return fmt.Errorf("usage: CLUSTER=x %s {create,delete}", os.Args[0])
+		return fmt.Errorf("usage: CLUSTER=x %s {create,createApp,deleteApp,delete}", os.Args[0])
 	}
 
 	if err := env.ValidateVars(Cluster); err != nil {
@@ -59,6 +59,10 @@ func run(ctx context.Context, log *logrus.Entry) error {
 	switch strings.ToLower(os.Args[1]) {
 	case "create":
 		return c.Create(ctx, vnetResourceGroup, clusterName, osClusterVersion)
+	case "createapp":
+		return c.CreateApp(ctx, clusterName)
+	case "deleteapp":
+		return c.DeleteApp(ctx)
 	case "delete":
 		return c.Delete(ctx, vnetResourceGroup, clusterName)
 	default:

hack/e2e/run-rp-and-e2e.sh

@@ -211,9 +211,31 @@ delete_e2e_cluster() {
         ./cluster delete
     else
         go run ./hack/cluster delete
+        go run ./hack/cluster deleteApp
     fi
 }
 
+get_cluster_sp() {
+    echo "########## Downloading SP secrets ##########"
+
+    az keyvault secret download --vault-name=aro-e2e-principals \
+        --name=aro-v4-e2e-devops-spn-1-app-id \
+        --file=secrets/app-id
+    az keyvault secret download --vault-name=aro-e2e-principals \
+        --name=aro-v4-e2e-devops-spn-1-sp-id \
+        --file=secrets/sp-id
+    az keyvault secret download --vault-name=aro-e2e-principals \
+        --name=aro-v4-e2e-devops-spn-1-secret-value \
+        --file=secrets/secret-value
+
+    echo -e -n "\nexport AZURE_CLUSTER_SERVICE_PRINCIPAL_ID=" >>secrets/env
+    cat secrets/sp-id >>secrets/env
+    echo -e -n "\nexport AZURE_CLUSTER_APP_ID=" >>secrets/env
+    cat secrets/app-id >>secrets/env
+    echo -e -n "\nexport AZURE_CLUSTER_APP_SECRET=" >>secrets/env
+    cat secrets/secret-value >>secrets/env
+}
+
 # TODO: CLUSTER and is also recalculated in multiple places
 # in the billing pipelines :-(
 

pkg/env/env.go

@@ -15,6 +15,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
 	mgmtcompute "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-06-01/compute"
 	"github.com/Azure/go-autorest/autorest"
+	"github.com/hashicorp/go-multierror"
 	"github.com/sirupsen/logrus"
 
 	"github.com/Azure/ARO-RP/pkg/proxy"
@@ -123,10 +124,12 @@ func IsCI() bool {
 // if it does not exist an environment variable with that name, it will return an error.
 // Otherwise it returns nil.
 func ValidateVars(vars ...string) error {
+	var err error
+
 	for _, envName := range vars {
 		if envValue, found := os.LookupEnv(envName); !found || envValue == "" {
-			return fmt.Errorf("environment variable %q unset", envName)
+			err = multierror.Append(fmt.Errorf("environment variable %q unset", envName), err)
 		}
 	}
-	return nil
+	return err
 }

pkg/util/cluster/cluster.go

@@ -139,6 +139,33 @@ func New(log *logrus.Entry, environment env.Core, ci bool) (*Cluster, error) {
 	return c, nil
 }
 
+func (c *Cluster) CreateApp(ctx context.Context, clusterName string) error {
+	c.log.Infof("creating AAD application")
+	appID, appSecret, err := c.createApplication(ctx, "aro-"+clusterName)
+	if err != nil {
+		return err
+	}
+
+	c.log.Infof("creating service principal")
+	spID, err := c.createServicePrincipal(ctx, appID)
+	if err != nil {
+		return err
+	}
+
+	return os.WriteFile("clusterapp.env", []byte(fmt.Sprintf("AZURE_CLUSTER_SERVICE_PRINCIPAL_ID=%s\nAZURE_CLUSTER_APP_ID=%s\nAZURE_CLUSTER_APP_SECRET=%s", spID, appID, appSecret)), 0o600)
+}
+
+func (c *Cluster) DeleteApp(ctx context.Context) error {
+	err := env.ValidateVars(
+		"AZURE_CLUSTER_APP_ID",
+	)
+	if err != nil {
+		return err
+	}
+
+	return c.deleteApplication(ctx, os.Getenv("AZURE_CLUSTER_APP_ID"))
+}
+
 func (c *Cluster) Create(ctx context.Context, vnetResourceGroup, clusterName string, osClusterVersion string) error {
 	clusterGet, err := c.openshiftclustersv20230904.Get(ctx, vnetResourceGroup, clusterName)
 	if err == nil {
@@ -149,22 +176,20 @@ func (c *Cluster) Create(ctx context.Context, vnetResourceGroup, clusterName str
 		return nil
 	}
 
+	err = env.ValidateVars(
+		"AZURE_FP_SERVICE_PRINCIPAL_ID",
+		"AZURE_CLUSTER_SERVICE_PRINCIPAL_ID",
+		"AZURE_CLUSTER_APP_ID",
+		"AZURE_CLUSTER_APP_SECRET",
+	)
+	if err != nil {
+		return err
+	}
+
 	fpSPID := os.Getenv("AZURE_FP_SERVICE_PRINCIPAL_ID")
-
+	spID := os.Getenv("AZURE_CLUSTER_SERVICE_PRINCIPAL_ID")
-	if fpSPID == "" {
+	appID := os.Getenv("AZURE_CLUSTER_APP_ID")
-		return fmt.Errorf("fp service principal id is not found")
+	appSecret := os.Getenv("AZURE_CLUSTER_APP_SECRET")
-	}
-
-	c.log.Infof("creating AAD application")
-	appID, appSecret, err := c.createApplication(ctx, "aro-"+clusterName)
-	if err != nil {
-		return err
-	}
-
-	spID, err := c.createServicePrincipal(ctx, appID)
-	if err != nil {
-		return err
-	}
 
 	visibility := api.VisibilityPublic
 
@@ -194,9 +219,6 @@ func (c *Cluster) Create(ctx context.Context, vnetResourceGroup, clusterName str
 	}
 
 	addressPrefix, masterSubnet, workerSubnet := c.generateSubnets()
-	if err != nil {
-		return err
-	}
 
 	var kvName string
 	if len(vnetResourceGroup) > 10 {
@@ -363,16 +385,12 @@ func (c *Cluster) Delete(ctx context.Context, vnetResourceGroup, clusterName str
 
 	oc, err := c.openshiftclustersv20200430.Get(ctx, vnetResourceGroup, clusterName)
 	if err == nil {
+		c.log.Print("deleting role assignments")
 		err = c.deleteRoleAssignments(ctx, vnetResourceGroup, *oc.OpenShiftClusterProperties.ServicePrincipalProfile.ClientID)
 		if err != nil {
 			errs = append(errs, err)
 		}
 
-		err = c.deleteApplication(ctx, *oc.OpenShiftClusterProperties.ServicePrincipalProfile.ClientID)
-		if err != nil {
-			errs = append(errs, err)
-		}
-
 		c.log.Print("deleting cluster")
 		err = c.openshiftclustersv20200430.DeleteAndWait(ctx, vnetResourceGroup, clusterName)
 		if err != nil {