az cli client cleanup (Azure#434)

cmd/setupgh.go

@@ -34,7 +34,6 @@ application and service principle, and will configure that application to trust
 			ctx := cmd.Context()
 
 			gh := providers.NewGhClient()
-			providers.EnsureAzCli()
 
 			azCred, err := cred.GetCred()
 			if err != nil {
@@ -46,14 +45,14 @@ application and service principle, and will configure that application to trust
 			}
 			sc.AzClient = az
 
-			err = fillSetUpConfig(sc, gh)
+			err = fillSetUpConfig(sc, gh, az)
 			if err != nil {
 				return fmt.Errorf("filling setup config: %w", err)
 			}
 
 			s := spinner.CreateSpinner("--> Setting up Github OIDC...")
 			s.Start()
-			err = runProviderSetUp(ctx, sc, s, gh)
+			err = runProviderSetUp(ctx, sc, s, gh, az)
 			s.Stop()
 			if err != nil {
 				return err
@@ -75,7 +74,7 @@ application and service principle, and will configure that application to trust
 	return cmd
 }
 
-func fillSetUpConfig(sc *providers.SetUpCmd, gh providers.GhClient) error {
+func fillSetUpConfig(sc *providers.SetUpCmd, gh providers.GhClient, az providers.AzClientInterface) error {
 	if sc.TenantId == "" {
 		tenandId, err := providers.PromptTenantId(sc.AzClient, context.Background())
 		if err != nil {
@@ -109,12 +108,12 @@ func fillSetUpConfig(sc *providers.SetUpCmd, gh providers.GhClient) error {
 
 	if sc.SubscriptionID == "" {
 		if strings.ToLower(sc.Provider) == "azure" {
-			currentSub, err := providers.GetCurrentAzSubscriptionLabel()
+			currentSub, err := az.GetCurrentAzSubscriptionLabel()
 			if err != nil {
 				return fmt.Errorf("getting current subscription ID: %w", err)
 			}
 
-			subLabels, err := providers.GetAzSubscriptionLabels()
+			subLabels, err := az.GetAzSubscriptionLabels()
 			if err != nil {
 				return fmt.Errorf("getting subscription labels: %w", err)
 			}
@@ -183,17 +182,17 @@ func toValidAppName(name string) (string, error) {
 
 	// lowercase the name
 	cleanedName = strings.ToLower(builder.String())
-	if err := ValidateAppName(cleanedName);err != nil {
+	if err := ValidateAppName(cleanedName); err != nil {
 		return "", fmt.Errorf("app name '%s' could not be converted to a valid name: %w", name, err)
 	}
 	return cleanedName, nil
 }
 
-func runProviderSetUp(ctx context.Context, sc *providers.SetUpCmd, s spinner.Spinner, gh providers.GhClient) error {
+func runProviderSetUp(ctx context.Context, sc *providers.SetUpCmd, s spinner.Spinner, gh providers.GhClient, az providers.AzClientInterface) error {
 	provider := strings.ToLower(sc.Provider)
 	if provider == "azure" {
 		// call azure provider logic
-		return providers.InitiateAzureOIDCFlow(ctx, sc, s, gh)
+		return providers.InitiateAzureOIDCFlow(ctx, sc, s, gh, az)
 
 	} else {
 		// call logic for user-submitted provider
@@ -223,7 +222,7 @@ func PromptAppName(az providers.AzClientInterface, defaultAppName string) (strin
 		return "", err
 	}
 
-	if providers.AzAppExists(appName) {
+	if az.AzAppExists(appName) {
 		confirmAppExistsPrompt := promptui.Prompt{
 			Label:     "An app with this name already exists. Would you like to use it?",
 			IsConfirm: true,

cmd/setupgh_test.go

@@ -22,9 +22,10 @@ func TestSetUpConfig(t *testing.T) {
 	s := spinner.CreateSpinner("--> Setting up Github OIDC...")
 
 	gh := &providers.GhCliClient{}
-	fillSetUpConfig(mockSetUpCmd, gh)
+	az := &providers.AzClient{}
+	fillSetUpConfig(mockSetUpCmd, gh, az)
 
-	err := runProviderSetUp(ctx, mockSetUpCmd, s, gh)
+	err := runProviderSetUp(ctx, mockSetUpCmd, s, gh, az)
 
 	assert.True(t, err == nil)
 }

pkg/prompts/prompts.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/manifoldco/promptui"
 	log "github.com/sirupsen/logrus"
+	"k8s.io/apimachinery/pkg/util/validation"
 
 	"github.com/Azure/draft/pkg/config"
 )
@@ -141,28 +142,10 @@ func NoBlankStringValidator(s string) error {
 
 // Validator for App name
 func appNameValidator(name string) error {
-	if name == "" {
-		return fmt.Errorf("application name cannot be empty")
+	errors := validation.IsDNS1123Label(name)
+	if errors != nil {
+		return fmt.Errorf("invalid app name: %s", strings.Join(errors, ", "))
 	}
-
-	if !unicode.IsLetter(rune(name[0])) && !unicode.IsDigit(rune(name[0])) {
-		return fmt.Errorf("application name must start with a letter or digit")
-	}
-
-	if name[len(name)-1] == '-' || name[len(name)-1] == '_' || name[len(name)-1] == '.' {
-		return fmt.Errorf("application name must end with a letter or digit")
-	}
-
-	for _, r := range name {
-		if !unicode.IsLetter(r) && !unicode.IsDigit(r) && r != '-' && r != '_' && r != '.' {
-			return fmt.Errorf("application name can only contain letters, digits, '-', '_', and '.'")
-		}
-	}
-
-	if len(name) > 63 {
-		return fmt.Errorf("application name cannot be longer than 63 characters")
-	}
-
 	return nil
 }
 
@@ -312,9 +295,9 @@ func getCurrentDirName() (string, error) {
 func sanitizeAppName(name string) string {
 	var builder strings.Builder
 
-	// Remove all characters except alphanumeric, '-', '_', '.'
+	// Remove all characters except alphanumeric, '-',  '.'
 	for _, r := range name {
-		if unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-' || r == '_' || r == '.' {
+		if unicode.IsLetter(r) || unicode.IsDigit(r) || strings.ContainsRune("-.", r) {
 			builder.WriteRune(r)
 		}
 	}

pkg/providers/az-client.go

@@ -15,9 +15,27 @@ import (
 
 //go:generate mockgen -source=./az-client.go -destination=./mock/az-client.go .
 type AzClientInterface interface {
+	AssignSpRole(ctx context.Context, subscriptionId, resourceGroup, servicePrincipalObjectID, roleId string) error
+	AzAcrExists(acrName string) bool
+	AzAksExists(aksName string, resourceGroup string) bool
+	AzAppExists(appName string) bool
+	CreateAzApp(appName string) (string, error)
+	CreateServicePrincipal(appId string) (string, error)
+	EnsureAzCli() error
+	EnsureAzCliLoggedIn() error
+	GetAzCliVersion() (string, error)
+	GetAzSubscriptionLabels() ([]SubLabel, error)
+	GetAzUpgrade() string
+	GetCurrentAzSubscriptionLabel() (SubLabel, error)
+	GetServicePrincipal(appId string) (string, error)
+	IsLoggedInToAz() bool
+	IsSubscriptionIdValid(subscriptionId string) error
+	IsValidResourceGroup(subscriptionId string, resourceGroup string) error
 	ListResourceGroups(ctx context.Context, subscriptionID string) ([]armresources.ResourceGroup, error)
 	ListTenants(ctx context.Context) ([]armsubscription.TenantIDDescription, error)
-	assignSpRole(ctx context.Context, subscriptionId, resourceGroup, servicePrincipalObjectID, roleId string) error
+	LogInToAz() error
+	UpgradeAzCli()
+	ValidateAzCliInstalled() error
 }
 
 // assert AzClient implements AzClientInterface
@@ -31,11 +49,17 @@ type AzClient struct {
 	TenantClient        *armsubscription.TenantsClient
 	RoleAssignClient    *armauthorization.RoleAssignmentsClient
 	ResourceGroupClient *armresources.ResourceGroupsClient
+	CommandRunner       CommandRunner
 }
 
 func NewAzClient(cred *azidentity.DefaultAzureCredential) (*AzClient, error) {
 	azClient := &AzClient{
-		Credential: cred,
+		Credential:    cred,
+		CommandRunner: &DefaultCommandRunner{},
+	}
+	err := azClient.EnsureAzCli()
+	if err != nil {
+		log.Fatal(err)
 	}
 	return azClient, nil
 }
@@ -103,7 +127,7 @@ func (az *AzClient) ListTenants(ctx context.Context) ([]armsubscription.TenantID
 	return tenants, nil
 }
 
-func (az *AzClient) assignSpRole(ctx context.Context, subscriptionId, resourceGroup, servicePrincipalObjectID, roleId string) error {
+func (az *AzClient) AssignSpRole(ctx context.Context, subscriptionId, resourceGroup, servicePrincipalObjectID, roleId string) error {
 	log.Debug("Assigning contributor role to service principal...")
 	if az.RoleAssignClient == nil {
 		c, err := armauthorization.NewRoleAssignmentsClient(subscriptionId, az.Credential, nil)