Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Big Fixes for v0.17-rc1 #5127

Merged
merged 2 commits into from
Feb 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,7 @@ stages:
--provider-aws-region $AWS_REGION
dist/rad credential register azure --client-id $INTEGRATION_TEST_SP_APP_ID \
--client-secret $INTEGRATION_TEST_SP_PASSWORD \
--tenant-id $INTEGRATION_TEST_TENANT_ID \
--subscription ${{ variables.INTEGRATION_TEST_SUBSCRIPTION_ID }} \
--resource-group $INTEGRATION_TEST_RESOURCE_GROUP_NAME
--tenant-id $INTEGRATION_TEST_TENANT_ID
dist/rad credential register aws \
--access-key-id "$(AWS_ACCESS_KEY_ID)" \
--secret-access-key "$(AWS_SECRET_ACCESS_KEY)"
Expand Down
14 changes: 11 additions & 3 deletions pkg/cli/cmd/credential/common/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (

// Used in tests
const (
AzureCloudProvider = "Azure"
AWSCloudProvider = "AWS"
SelectExistingEnvironmentPrompt = "Select an existing environment or create a new one"
SelectExistingEnvironmentCreateSentinel = "[create new]"
EnterEnvironmentNamePrompt = "Enter an environment name"
Expand All @@ -26,12 +28,18 @@ const (
AWSCredentialID = "/planes/aws/aws/providers/System.AWS/credentials/%s"
)

var (
supportedProviders = []string{AzureCloudProvider, AWSCloudProvider}
)

func ValidateCloudProviderName(name string) error {
if strings.EqualFold(name, "azure") {
return nil
for _, provider := range (supportedProviders) {
if strings.EqualFold(name, provider){
return nil
}
}

return &cli.FriendlyError{Message: fmt.Sprintf("Cloud provider type %q is not supported. Supported types: azure.", name)}
return &cli.FriendlyError{Message: fmt.Sprintf("Cloud provider type %q is not supported. ", strings.Join(supportedProviders, " "))}
}

// SelectExistingEnvironment prompts the user to select from existing environments (with the option to create a new one).
Expand Down
39 changes: 2 additions & 37 deletions pkg/cli/cmd/credential/register/azure/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
cli_credential "github.com/project-radius/radius/pkg/cli/credential"
"github.com/project-radius/radius/pkg/cli/framework"
"github.com/project-radius/radius/pkg/cli/output"
"github.com/project-radius/radius/pkg/cli/prompt"
"github.com/project-radius/radius/pkg/cli/workspaces"
ucp "github.com/project-radius/radius/pkg/ucp/api/v20220901privatepreview"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -46,7 +45,7 @@ calling 'rad provider create azure'.
` + common.LongDescriptionBlurb,
Example: `
# Register (Add or update) cloud provider credential for Azure with service principal authentication
rad credential register azure --client-id <client id/app id> --client-secret <client secret/password> --tenant-id <tenant id> --subscription <subscription id> --resource-group <resource group name>
rad credential register azure --client-id <client id/app id> --client-secret <client secret/password> --tenant-id <tenant id>
`,
Args: cobra.ExactArgs(0),
RunE: framework.RunCommand(runner),
Expand All @@ -64,12 +63,6 @@ rad credential register azure --client-id <client id/app id> --client-secret <cl
cmd.Flags().String("tenant-id", "", "The tenant id of an Azure service principal.")
_ = cmd.MarkFlagRequired("tenant-id")

cmd.Flags().String("subscription", "", "The subscription id of the target Azure subscription. The subscription id will be stored in local configuration and used by 'rad deploy'.")
_ = cmd.MarkFlagRequired("subscription")

cmd.Flags().String("resource-group", "", "The resource group name of an existing Azure resource group. The resource group will be stored in local configuration and used by 'rad deploy'.")
_ = cmd.MarkFlagRequired("resource-group")

return cmd, runner
}

Expand All @@ -84,10 +77,7 @@ type Runner struct {
ClientID string
ClientSecret string
TenantID string
//TODO: move scope components out to provider commands
SubscriptionID string
ResourceGroup string
KubeContext string
KubeContext string
}

// NewRunner creates a new instance of the `rad credential register azure` runner.
Expand Down Expand Up @@ -130,25 +120,10 @@ func (r *Runner) Validate(cmd *cobra.Command, args []string) error {
if err != nil {
return err
}
subscriptionID, err := cmd.Flags().GetString("subscription")
if err != nil {
return err
}
resourceGroup, err := cmd.Flags().GetString("resource-group")
if err != nil {
return err
}

r.ClientID = clientID
r.ClientSecret = clientSecret
r.TenantID = tenantID
r.SubscriptionID = subscriptionID
r.ResourceGroup = resourceGroup

valid, message, _ := prompt.UUIDv4Validator(r.SubscriptionID)
if !valid {
return &cli.FriendlyError{Message: fmt.Sprintf("Subscription id %q is invalid: %s", r.SubscriptionID, message)}
}

kubeContext, ok := r.Workspace.KubernetesContext()
if !ok {
Expand Down Expand Up @@ -191,15 +166,5 @@ func (r *Runner) Run(ctx context.Context) error {
if err != nil {
return err
}

// 2) Update local config (all matching workspaces) to remove the scope
// TODO: move updating scope to provider commands
err = cli.EditWorkspaces(ctx, r.ConfigHolder.Config, func(section *cli.WorkspaceSection) error {
cli.UpdateAzProvider(section, workspaces.AzureProvider{SubscriptionID: r.SubscriptionID, ResourceGroup: r.ResourceGroup}, r.KubeContext)
return nil
})
if err != nil {
return err
}
return nil
}
82 changes: 4 additions & 78 deletions pkg/cli/cmd/credential/register/azure/azure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ func Test_Validate(t *testing.T) {
"--client-id", "abcd",
"--client-secret", "efgh",
"--tenant-id", "ijkl",
"--subscription", "E3955194-FC78-40A8-8143-C5D8DCDC45C5",
"--resource-group", "cool-group",
},
ExpectedValid: true,
ConfigHolder: framework.ConfigHolder{Config: configWithWorkspace},
Expand All @@ -52,8 +50,6 @@ func Test_Validate(t *testing.T) {
"--client-id", "abcd",
"--client-secret", "efgh",
"--tenant-id", "ijkl",
"--subscription", "E3955194-FC78-40A8-8143-C5D8DCDC45C5",
"--resource-group", "cool-group",
},
ExpectedValid: false,
ConfigHolder: framework.ConfigHolder{Config: radcli.LoadEmptyConfig(t)},
Expand All @@ -65,8 +61,6 @@ func Test_Validate(t *testing.T) {
"--client-id", "abcd",
"--client-secret", "efgh",
"--tenant-id", "ijkl",
"--subscription", "E3955194-FC78-40A8-8143-C5D8DCDC45C5",
"--resource-group", "cool-group",
},
ExpectedValid: false,
ConfigHolder: framework.ConfigHolder{Config: configWithWorkspace},
Expand All @@ -76,8 +70,6 @@ func Test_Validate(t *testing.T) {
Input: []string{
"--client-secret", "efgh",
"--tenant-id", "ijkl",
"--subscription", "E3955194-FC78-40A8-8143-C5D8DCDC45C5",
"--resource-group", "cool-group",
},
ExpectedValid: false,
ConfigHolder: framework.ConfigHolder{Config: configWithWorkspace},
Expand All @@ -87,8 +79,6 @@ func Test_Validate(t *testing.T) {
Input: []string{
"--client-id", "abcd",
"--tenant-id", "ijkl",
"--subscription", "E3955194-FC78-40A8-8143-C5D8DCDC45C5",
"--resource-group", "cool-group",
},
ExpectedValid: false,
ConfigHolder: framework.ConfigHolder{Config: configWithWorkspace},
Expand All @@ -98,8 +88,6 @@ func Test_Validate(t *testing.T) {
Input: []string{
"--client-id", "abcd",
"--client-secret", "efgh",
"--subscription", "E3955194-FC78-40A8-8143-C5D8DCDC45C5",
"--resource-group", "cool-group",
},
ExpectedValid: false,
ConfigHolder: framework.ConfigHolder{Config: configWithWorkspace},
Expand All @@ -109,32 +97,6 @@ func Test_Validate(t *testing.T) {
Input: []string{
"--client-id", "abcd",
"--client-secret", "efgh",
"--tenant-id", "ijkl",
"--resource-group", "cool-group",
},
ExpectedValid: false,
ConfigHolder: framework.ConfigHolder{Config: configWithWorkspace},
},
{
Name: "Azure command without resource group",
Input: []string{
"letsgoooooo",
"--client-id", "abcd",
"--client-secret", "efgh",
"--tenant-id", "ijkl",
"--subscription", "E3955194-FC78-40A8-8143-C5D8DCDC45C5",
},
ExpectedValid: false,
ConfigHolder: framework.ConfigHolder{Config: configWithWorkspace},
},
{
Name: "Azure command with invalid subscription id",
Input: []string{
"--client-id", "abcd",
"--client-secret", "efgh",
"--tenant-id", "ijkl",
"--subscription", "E3955194-FC78-40A8-8143-C5D8DCDC45C5invalid",
"--resource-group", "cool-group",
},
ExpectedValid: false,
ConfigHolder: framework.ConfigHolder{Config: configWithWorkspace},
Expand Down Expand Up @@ -170,29 +132,13 @@ func Test_Run(t *testing.T) {
"context": "my-context",
},
Source: workspaces.SourceUserConfig,

// Will be over-written
ProviderConfig: workspaces.ProviderConfig{
Azure: &workspaces.AzureProvider{
SubscriptionID: "FE3955194-FC78-40A8-8143-C5D8DCDC45C5",
ResourceGroup: "another-cool-group",
},
},
},
"c": {
Connection: map[string]any{
"kind": workspaces.KindKubernetes,
"context": "my-other-context",
},
Source: workspaces.SourceUserConfig,

// Will be left-alone
ProviderConfig: workspaces.ProviderConfig{
Azure: &workspaces.AzureProvider{
SubscriptionID: "FE3955194-FC78-40A8-8143-C5D8DCDC45C5",
ResourceGroup: "another-cool-group",
},
},
},
},
},
Expand Down Expand Up @@ -241,12 +187,10 @@ func Test_Run(t *testing.T) {
},
Format: "table",

ClientID: "cool-client-id",
ClientSecret: "cool-client-secret",
TenantID: "cool-tenant-id",
SubscriptionID: "E3955194-FC78-40A8-8143-C5D8DCDC45C5",
ResourceGroup: "cool-resource-group",
KubeContext: "my-context",
ClientID: "cool-client-id",
ClientSecret: "cool-client-secret",
TenantID: "cool-tenant-id",
KubeContext: "my-context",
}

err = runner.Run(context.Background())
Expand All @@ -270,12 +214,6 @@ func Test_Run(t *testing.T) {
"context": "my-context",
},
Source: workspaces.SourceUserConfig,
ProviderConfig: workspaces.ProviderConfig{
Azure: &workspaces.AzureProvider{
SubscriptionID: "E3955194-FC78-40A8-8143-C5D8DCDC45C5",
ResourceGroup: "cool-resource-group",
},
},
},
"b": {
Name: "b",
Expand All @@ -284,12 +222,6 @@ func Test_Run(t *testing.T) {
"context": "my-context",
},
Source: workspaces.SourceUserConfig,
ProviderConfig: workspaces.ProviderConfig{
Azure: &workspaces.AzureProvider{
SubscriptionID: "E3955194-FC78-40A8-8143-C5D8DCDC45C5",
ResourceGroup: "cool-resource-group",
},
},
},
"c": {
Name: "c",
Expand All @@ -298,12 +230,6 @@ func Test_Run(t *testing.T) {
"context": "my-other-context",
},
Source: workspaces.SourceUserConfig,
ProviderConfig: workspaces.ProviderConfig{
Azure: &workspaces.AzureProvider{
SubscriptionID: "FE3955194-FC78-40A8-8143-C5D8DCDC45C5",
ResourceGroup: "another-cool-group",
},
},
},
},
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/cli/cmd/credential/show/show_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ func Test_Validate(t *testing.T) {
Config: configWithWorkspace,
},
},
{
Name: "Valid Show AWS Command",
Input: []string{"aws"},
ExpectedValid: true,
ConfigHolder: framework.ConfigHolder{
ConfigFilePath: "",
Config: configWithWorkspace,
},
},
}
radcli.SharedValidateValidation(t, NewCommand, testcases)
}
Expand Down
10 changes: 4 additions & 6 deletions pkg/cli/cmd/radInit/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ import (
)

const (
azureCloudProvider = "Azure"
awsCloudProvider = "AWS"
backNavigator = "[back]"
backNavigator = "[back]"

confirmCloudProviderPrompt = "Add cloud providers for cloud resources?"
confirmReinstallRadiusPrompt = "Would you like to reinstall Radius control plane?"
Expand Down Expand Up @@ -286,12 +284,12 @@ func (r *Runner) Validate(cmd *cobra.Command, args []string) error {
return &cli.FriendlyError{Message: "Error reading cloud provider"}
}
switch cloudProvider {
case azureCloudProvider:
case common.AzureCloudProvider:
r.AzureCloudProvider, err = r.SetupInterface.ParseAzureProviderArgs(cmd, true, r.Prompter)
if err != nil {
return err
}
case awsCloudProvider:
case common.AWSCloudProvider:
r.AwsCloudProvider, err = r.SetupInterface.ParseAWSProviderArgs(cmd, true, r.Prompter)
if err != nil {
return err
Expand Down Expand Up @@ -513,7 +511,7 @@ func selectKubeContext(currentContext string, kubeContexts map[string]*api.Conte

// Selects the cloud provider, returns -1 if back and -2 if not supported
func selectCloudProvider(prompter prompt.Interface) (string, error) {
values := []string{azureCloudProvider, awsCloudProvider, backNavigator}
values := []string{common.AzureCloudProvider, common.AWSCloudProvider, backNavigator}
return prompter.GetListInput(values, selectCloudProviderPrompt)
}

Expand Down
Loading