From cc34a129e28fba08533481c749c82f7d87f0d017 Mon Sep 17 00:00:00 2001 From: Magnus Kulke Date: Wed, 30 Aug 2023 12:33:15 +0200 Subject: [PATCH] test: misc enhancements to the azure e2e test code - Remove option to pass secrets to test run since leakage of secrets in the CI is a risk and federated credentials work well on github actions we remove the option to pass a CLIENT_SECRET to a test run. Local runs can keep using cli auth. - Parallize the tests it will speed up a test run and is a valid test path. there are no dependencies between the test cases, so flakes due to this would imply a bug we want to be aware of. - There is a misnomer in the ssh key parameter. - There is no need to query the API for the rg name in the cloud asserter, since we have the name in all cases. Signed-off-by: Magnus Kulke --- .github/workflows/azure-e2e-test.yml | 1 - test/e2e/azure_test.go | 32 ++++++---------- test/provisioner/provision_azure.go | 35 ++--------------- .../provision_azure_cli_auth.properties | 5 +-- .../provision_azure_initializer.go | 38 ++++++------------- 5 files changed, 29 insertions(+), 82 deletions(-) diff --git a/.github/workflows/azure-e2e-test.yml b/.github/workflows/azure-e2e-test.yml index 28a7b2b4e..01e2fd8e0 100644 --- a/.github/workflows/azure-e2e-test.yml +++ b/.github/workflows/azure-e2e-test.yml @@ -113,7 +113,6 @@ jobs: SSH_KEY_ID="id_rsa.pub" AZURE_IMAGE_ID="$AZURE_IMAGE_ID" IS_CI_MANAGED_CLUSTER="true" - AZURE_CLI_AUTH="true" MANAGED_IDENTITY_NAME="${{ secrets.AZURE_MANAGED_IDENTITY_NAME}}" CAA_IMAGE="${CAA_IMAGE}" EOF diff --git a/test/e2e/azure_test.go b/test/e2e/azure_test.go index 9ff860629..635482cca 100644 --- a/test/e2e/azure_test.go +++ b/test/e2e/azure_test.go @@ -10,44 +10,35 @@ import ( "strings" "testing" - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources" pv "github.com/confidential-containers/cloud-api-adaptor/test/provisioner" log "github.com/sirupsen/logrus" ) +// AzureCloudAssert implements the CloudAssert interface for azure. +type AzureCloudAssert struct{} + +var assert = &AzureCloudAssert{} + func TestDeletePodAzure(t *testing.T) { - assert := AzureCloudAssert{ - group: pv.AzureProps.ResourceGroup, - } + t.Parallel() doTestDeleteSimplePod(t, assert) } func TestCreateSimplePodAzure(t *testing.T) { - assert := AzureCloudAssert{ - group: pv.AzureProps.ResourceGroup, - } + t.Parallel() doTestCreateSimplePod(t, assert) } func TestCreatePodWithConfigMapAzure(t *testing.T) { - assert := AzureCloudAssert{ - group: pv.AzureProps.ResourceGroup, - } + t.Parallel() doTestCreatePodWithConfigMap(t, assert) } func TestCreatePodWithSecretAzure(t *testing.T) { - assert := AzureCloudAssert{ - group: pv.AzureProps.ResourceGroup, - } + t.Parallel() doTestCreatePodWithSecret(t, assert) } -// AzureCloudAssert implements the CloudAssert interface for azure. -type AzureCloudAssert struct { - group *armresources.ResourceGroup -} - func checkVMExistence(resourceGroupName, prefixName string) bool { pager := pv.AzureProps.ManagedVmClient.NewListPager(resourceGroupName, nil) @@ -72,10 +63,11 @@ func checkVMExistence(resourceGroupName, prefixName string) bool { func (c AzureCloudAssert) HasPodVM(t *testing.T, id string) { pod_vm_prefix := "podvm-" + id - if checkVMExistence(*c.group.Name, pod_vm_prefix) { + rg := pv.AzureProps.ResourceGroupName + if checkVMExistence(rg, pod_vm_prefix) { log.Infof("VM found in resource group") } else { - log.Infof("Virtual machine %s not found in resource group ", id) + log.Infof("Virtual machine %s not found in resource group %s", id, rg) t.Error("PodVM was not created") } } diff --git a/test/provisioner/provision_azure.go b/test/provisioner/provision_azure.go index 47f350208..bb04258ff 100644 --- a/test/provisioner/provision_azure.go +++ b/test/provisioner/provision_azure.go @@ -37,15 +37,13 @@ func createResourceGroup() error { if AzureProps.IsCIManaged { log.Infof("Resource group %q is CI managed. No need to create new one manually.", AzureProps.ResourceGroupName) - resp, err := AzureProps.ResourceGroupClient.Get(context.Background(), AzureProps.ResourceGroupName, nil) + _, err := AzureProps.ResourceGroupClient.Get(context.Background(), AzureProps.ResourceGroupName, nil) if err != nil { err = fmt.Errorf("getting resource group %s: %w", AzureProps.ResourceGroupName, err) log.Errorf("%v", err) return err } - AzureProps.ResourceGroup = &resp.ResourceGroup - return nil } @@ -54,15 +52,13 @@ func createResourceGroup() error { } log.Infof("Creating Resource group %s.\n", AzureProps.ResourceGroupName) - resourceGroupResp, err := AzureProps.ResourceGroupClient.CreateOrUpdate(context.Background(), AzureProps.ResourceGroupName, newRG, nil) + _, err := AzureProps.ResourceGroupClient.CreateOrUpdate(context.Background(), AzureProps.ResourceGroupName, newRG, nil) if err != nil { err = fmt.Errorf("creating resource group %s: %w", AzureProps.ResourceGroupName, err) log.Errorf("%v", err) return err } - AzureProps.ResourceGroup = &resourceGroupResp.ResourceGroup - log.Infof("Successfully Created Resource group %s.\n", AzureProps.ResourceGroupName) return nil } @@ -252,16 +248,6 @@ func (p *AzureCloudProvisioner) CreateCluster(ctx context.Context, cfg *envconf. }, } - // Enable service principal when not using the az CLI authentication method. - if !AzureProps.IsAzCliAuth { - spProfile := &armcontainerservice.ManagedClusterServicePrincipalProfile{ - ClientID: to.Ptr(AzureProps.ClientID), - Secret: to.Ptr(AzureProps.ClientSecret), - } - - managedcluster.Properties.ServicePrincipalProfile = spProfile - } - pollerResp, err := AzureProps.ManagedAksClient.BeginCreateOrUpdate( context.Background(), AzureProps.ResourceGroupName, @@ -370,21 +356,13 @@ func (p *AzureCloudProvisioner) GetProperties(ctx context.Context, cfg *envconf. "AZURE_RESOURCE_GROUP": AzureProps.ResourceGroupName, "CLUSTER_NAME": AzureProps.ClusterName, "AZURE_REGION": AzureProps.Location, - "SSH_KEY_ID": AzureProps.SshPrivateKey, + "SSH_KEY_ID": AzureProps.SSHKeyID, "SSH_USERNAME": AzureProps.SshUserName, "AZURE_IMAGE_ID": AzureProps.ImageID, "AZURE_SUBNET_ID": AzureProps.SubnetID, "AZURE_INSTANCE_SIZE": AzureProps.InstanceSize, } - if AzureProps.ClientSecret != "" { - props["AZURE_CLIENT_SECRET"] = AzureProps.ClientSecret - } - - if AzureProps.TenantID != "" { - props["AZURE_TENANT_ID"] = AzureProps.TenantID - } - return props } @@ -404,12 +382,7 @@ func isAzureKustomizeConfigMapKey(key string) bool { } func isAzureKustomizeSecretKey(key string) bool { - switch key { - case "AZURE_CLIENT_ID", "AZURE_CLIENT_SECRET", "AZURE_TENANT_ID": - return true - default: - return false - } + return key == "AZURE_CLIENT_ID" } func NewAzureInstallOverlay(installDir string) (InstallOverlay, error) { diff --git a/test/provisioner/provision_azure_cli_auth.properties b/test/provisioner/provision_azure_cli_auth.properties index 1e0175028..6d2966af4 100644 --- a/test/provisioner/provision_azure_cli_auth.properties +++ b/test/provisioner/provision_azure_cli_auth.properties @@ -1,7 +1,6 @@ AZURE_SUBSCRIPTION_ID="${AZURE_SUBSCRIPTION_ID}" -# use az ad sp create-for-rbac --role Contributor --scopes "/subscriptions/$SUBSCRIPTION_ID" --query "{ client_id: appId, client_secret: password, tenant_id: tenant }" -AZURE_TENANT_ID="${AZURE_TENANT_ID}" - +# retrieve client_id from federated credential +AZURE_CLIENT_ID="" RESOURCE_GROUP_NAME="e2e-test-rg" CLUSTER_NAME="e2e-test-cluster" LOCATION="eastus" diff --git a/test/provisioner/provision_azure_initializer.go b/test/provisioner/provision_azure_initializer.go index ae4c30787..b73e8f475 100644 --- a/test/provisioner/provision_azure_initializer.go +++ b/test/provisioner/provision_azure_initializer.go @@ -18,22 +18,18 @@ import ( ) type AzureProperties struct { - ResourceGroup *armresources.ResourceGroup SubscriptionID string ClientID string - ClientSecret string - TenantID string ResourceGroupName string ClusterName string Location string - SshPrivateKey string + SSHKeyID string SubnetName string VnetName string SubnetID string ImageID string SshUserName string ManagedIdentityName string - IsAzCliAuth bool IsCIManaged bool CaaImage string @@ -53,16 +49,15 @@ type AzureProperties struct { var AzureProps = &AzureProperties{} func initAzureProperties(properties map[string]string) error { - log.Trace("initazureProperties()") + log.Trace("initAzureProperties()") + AzureProps = &AzureProperties{ SubscriptionID: properties["AZURE_SUBSCRIPTION_ID"], ClientID: properties["AZURE_CLIENT_ID"], - ClientSecret: properties["AZURE_CLIENT_SECRET"], - TenantID: properties["AZURE_TENANT_ID"], ResourceGroupName: properties["RESOURCE_GROUP_NAME"], ClusterName: properties["CLUSTER_NAME"], Location: properties["LOCATION"], - SshPrivateKey: properties["SSH_KEY_ID"], + SSHKeyID: properties["SSH_KEY_ID"], ImageID: properties["AZURE_IMAGE_ID"], SubnetID: properties["AZURE_SUBNET_ID"], SshUserName: properties["SSH_USERNAME"], @@ -76,18 +71,6 @@ func initAzureProperties(properties map[string]string) error { AzureProps.IsCIManaged = true } - CliAuthStr := properties["AZURE_CLI_AUTH"] - AzureProps.IsAzCliAuth = false - if strings.EqualFold(CliAuthStr, "yes") || strings.EqualFold(CliAuthStr, "true") { - AzureProps.IsAzCliAuth = true - } - - AzureProps.VnetName = AzureProps.ClusterName + "_vnet" - AzureProps.SubnetName = AzureProps.ClusterName + "_subnet" - AzureProps.InstanceSize = "Standard_DC2as_v5" - AzureProps.NodeName = "caaaks" - AzureProps.OsType = "Ubuntu" - if AzureProps.SubscriptionID == "" { return errors.New("AZURE_SUBSCRIPTION_ID was not set.") } @@ -98,21 +81,22 @@ func initAzureProperties(properties map[string]string) error { if AzureProps.ClientID == "" { return errors.New("AZURE_CLIENT_ID was not set.") } - if AzureProps.ClientSecret == "" && !AzureProps.IsAzCliAuth { - return errors.New("AZURE_CLIENT_SECRET was not set") - } if AzureProps.Location == "" { return errors.New("LOCATION was not set.") } - if AzureProps.SshPrivateKey == "" { - return errors.New("SSH_KEY_ID was not set.") - } if AzureProps.ImageID == "" { return errors.New("AZURE_IMAGE_ID was not set.") } if AzureProps.ClusterName == "" { AzureProps.ClusterName = "e2e_test_cluster" } + + AzureProps.VnetName = AzureProps.ClusterName + "_vnet" + AzureProps.SubnetName = AzureProps.ClusterName + "_subnet" + AzureProps.InstanceSize = "Standard_DC2as_v5" + AzureProps.NodeName = "caaaks" + AzureProps.OsType = "Ubuntu" + if AzureProps.ResourceGroupName == "" { AzureProps.ResourceGroupName = AzureProps.ClusterName + "_rg" }