Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -805,12 +805,19 @@
"$type": "ObjectType",
"name": "ProvidersAws",
"properties": {
"scope": {
"accountId": {
"type": {
"$ref": "#/0"
},
"flags": 1,
"description": "Target scope for AWS resources to be deployed into. For example: '/planes/aws/aws/accounts/000000000000/regions/us-west-2'."
"description": "AWS account ID for AWS resources to be deployed into."
},
"region": {
"type": {
"$ref": "#/0"
},
"flags": 1,
"description": "AWS region for AWS resources to be deployed into."
}
}
},
Expand Down
41 changes: 41 additions & 0 deletions pkg/cli/cmd/env/show/preview/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ type EnvRecipes struct {
RecipeLocation string
}

// EnvProviders represents a provider and its properties for an environment.
type EnvProviders struct {
// Provider is the type of the provider (e.g., "azure", "aws", "kubernetes").
Provider string
// Properties contains the provider details in a comma-separated key-value format.
// e.g., "subscriptionId: 'sub-id', resourceGroupName: 'rg-name'" for azure provider."
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an extra period at the end of the example comment that should be removed for consistency with other comment lines.

Suggested change
// e.g., "subscriptionId: 'sub-id', resourceGroupName: 'rg-name'" for azure provider."
// e.g., "subscriptionId: 'sub-id', resourceGroupName: 'rg-name'" for azure provider

Copilot uses AI. Check for mistakes.
Properties string
}

// Runner is the runner implementation for the `rad env show` preview command.
type Runner struct {
ConfigHolder *framework.ConfigHolder
Expand Down Expand Up @@ -136,6 +145,32 @@ func (r *Runner) Run(ctx context.Context) error {
return err
}

envProviders := []EnvProviders{}
if resp.EnvironmentResource.Properties.Providers != nil {
if resp.EnvironmentResource.Properties.Providers.Azure != nil {
azureProvider := EnvProviders{
Provider: "azure",
}
azureProvider.Properties = "subscriptionId: '" + *resp.EnvironmentResource.Properties.Providers.Azure.SubscriptionID + "', resourceGroupName: '" + *resp.EnvironmentResource.Properties.Providers.Azure.ResourceGroupName + "'"
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential nil pointer dereference when accessing provider properties. If any of the pointer fields (SubscriptionID, ResourceGroupName, AccountID, Region, or Namespace) are nil, dereferencing them will cause a panic. Add nil checks before dereferencing these pointers or use helper functions to safely access pointer values.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using direct string concatenation with the + operator. For better performance and idiomatic Go code, use strings.Builder or fmt.Sprintf when building strings from multiple parts. This is particularly important since this code constructs display strings for potentially multiple providers.

Copilot generated this review using guidance from repository custom instructions.
envProviders = append(envProviders, azureProvider)
}

if resp.EnvironmentResource.Properties.Providers.Aws != nil {
awsProvider := EnvProviders{
Provider: "aws",
}
awsProvider.Properties = "accountId: '" + *resp.EnvironmentResource.Properties.Providers.Aws.AccountID + "', region: '" + *resp.EnvironmentResource.Properties.Providers.Aws.Region + "'"
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential nil pointer dereference when accessing AWS provider AccountID and Region fields. If either field is nil, dereferencing will cause a panic. Add nil checks before dereferencing these pointers or use helper functions to safely access pointer values.

Suggested change
awsProvider.Properties = "accountId: '" + *resp.EnvironmentResource.Properties.Providers.Aws.AccountID + "', region: '" + *resp.EnvironmentResource.Properties.Providers.Aws.Region + "'"
accountID := ""
if resp.EnvironmentResource.Properties.Providers.Aws.AccountID != nil {
accountID = *resp.EnvironmentResource.Properties.Providers.Aws.AccountID
}
region := ""
if resp.EnvironmentResource.Properties.Providers.Aws.Region != nil {
region = *resp.EnvironmentResource.Properties.Providers.Aws.Region
}
awsProvider.Properties = "accountId: '" + accountID + "', region: '" + region + "'"

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using direct string concatenation with the + operator. For better performance and idiomatic Go code, use strings.Builder or fmt.Sprintf when building strings from multiple parts. This is particularly important since this code constructs display strings for potentially multiple providers.

Copilot generated this review using guidance from repository custom instructions.
envProviders = append(envProviders, awsProvider)
}
if resp.EnvironmentResource.Properties.Providers.Kubernetes != nil {
k8sProvider := EnvProviders{
Provider: "kubernetes",
}
k8sProvider.Properties = "namespace: '" + *resp.EnvironmentResource.Properties.Providers.Kubernetes.Namespace + "'"
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential nil pointer dereference when accessing Kubernetes Namespace field. If the Namespace field is nil, dereferencing will cause a panic. Add nil checks before dereferencing this pointer or use helper functions to safely access pointer values.

Suggested change
k8sProvider.Properties = "namespace: '" + *resp.EnvironmentResource.Properties.Providers.Kubernetes.Namespace + "'"
namespace := ""
if resp.EnvironmentResource.Properties.Providers.Kubernetes.Namespace != nil {
namespace = *resp.EnvironmentResource.Properties.Providers.Kubernetes.Namespace
}
k8sProvider.Properties = "namespace: '" + namespace + "'"

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using direct string concatenation with the + operator. For better performance and idiomatic Go code, use strings.Builder or fmt.Sprintf when building strings from multiple parts. This is particularly important since this code constructs display strings for potentially multiple providers.

Copilot generated this review using guidance from repository custom instructions.
envProviders = append(envProviders, k8sProvider)
}
}
Comment on lines +148 to +172
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provider formatting logic is repetitive and could be refactored into a helper function to reduce code duplication. Consider extracting a function that takes provider type, field names, and values to construct the Properties string. This would make the code more maintainable and reduce the risk of inconsistent formatting across providers.

Copilot uses AI. Check for mistakes.

recipepackClient := r.RadiusCoreClientFactory.NewRecipePacksClient()
envRecipes := []EnvRecipes{}
for _, rp := range resp.EnvironmentResource.Properties.RecipePacks {
Expand Down Expand Up @@ -183,6 +218,12 @@ func (r *Runner) Run(ctx context.Context) error {
if err != nil {
return err
}
r.Output.LogInfo("")

err = r.Output.WriteFormatted(r.Format, envProviders, objectformats.GetProvidersForEnvironmentTableFormat())
if err != nil {
return err
}

r.Output.LogInfo("")
err = r.Output.WriteFormatted(r.Format, envRecipes, objectformats.GetRecipesForEnvironmentTableFormat())
Expand Down
34 changes: 34 additions & 0 deletions pkg/cli/cmd/env/show/preview/show_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,47 @@ func Test_Run(t *testing.T) {
RecipePacks: []*string{
to.Ptr("/planes/radius/local/resourceGroups/test-group/providers/Radius.Core/recipePacks/test-recipe-pack"),
},
Providers: &corerpv20250801.Providers{
Azure: &corerpv20250801.ProvidersAzure{
SubscriptionID: to.Ptr("test-subscription-id"),
ResourceGroupName: to.Ptr("test-resource-group"),
},
Aws: &corerpv20250801.ProvidersAws{
AccountID: to.Ptr("test-account-id"),
Region: to.Ptr("test-region"),
},
Kubernetes: &corerpv20250801.ProvidersKubernetes{
Namespace: to.Ptr("test-namespace"),
},
},
Comment on lines +121 to +133
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test coverage is missing for scenarios where provider fields could be nil. The new code in show.go dereferences pointer fields without nil checks (lines 154, 162, 169), which could cause panics. Add test cases covering environments with partially configured providers or nil pointer fields to ensure the code handles these edge cases gracefully.

Copilot uses AI. Check for mistakes.
},
},
Options: objectformats.GetResourceTableFormat(),
},
output.LogOutput{
Format: "",
},
output.FormattedOutput{
Format: "table",
Obj: []EnvProviders{
{
Provider: "azure",
Properties: "subscriptionId: 'test-subscription-id', resourceGroupName: 'test-resource-group'",
},
{
Provider: "aws",
Properties: "accountId: 'test-account-id', region: 'test-region'",
},
{
Provider: "kubernetes",
Properties: "namespace: 'test-namespace'",
},
},
Options: objectformats.GetProvidersForEnvironmentTableFormat(),
},
output.LogOutput{
Format: "",
},
output.FormattedOutput{
Format: "table",
Obj: []EnvRecipes{
Expand Down
12 changes: 6 additions & 6 deletions pkg/cli/cmd/env/update/preview/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package preview

import (
"context"
"fmt"

"github.com/spf13/cobra"

Expand All @@ -37,7 +36,6 @@ import (

const (
envNotFoundErrMessageFmt = "The environment %q does not exist. Please select a new environment and try again."
awsScopeTemplate = "/planes/aws/aws/accounts/%s/regions/%s"
)

// NewCommand creates an instance of the command and runner for the `rad env update` preview command.
Expand Down Expand Up @@ -87,8 +85,8 @@ rad env update myenv --clear-kubernetes
commonflags.AddResourceGroupFlag(cmd)
cmd.Flags().Bool(commonflags.ClearEnvAzureFlag, false, "Specify if azure provider needs to be cleared on env")
cmd.Flags().Bool(commonflags.ClearEnvAWSFlag, false, "Specify if aws provider needs to be cleared on env")
cmd.Flags().Bool(commonflags.ClearEnvKubernetesFlag, false, "Specify if kubernetes provider needs to be cleared on env (preview)")
cmd.Flags().StringArrayP("recipe-packs", "", []string{}, "Specify recipe packs to be added to the environment (preview)")
cmd.Flags().Bool(commonflags.ClearEnvKubernetesFlag, false, "Specify if kubernetes provider needs to be cleared on env (--preview)")
cmd.Flags().StringArrayP("recipe-packs", "", []string{}, "Specify recipe packs to be added to the environment (--preview)")
commonflags.AddAzureScopeFlags(cmd)
commonflags.AddAWSScopeFlags(cmd)
commonflags.AddKubernetesScopeFlags(cmd)
Expand Down Expand Up @@ -185,7 +183,8 @@ func (r *Runner) Validate(cmd *cobra.Command, args []string) error {
return err
}

r.providers.Aws.Scope = to.Ptr(fmt.Sprintf(awsScopeTemplate, awsAccountId, awsRegion))
r.providers.Aws.Region = to.Ptr(awsRegion)
r.providers.Aws.AccountID = to.Ptr(awsAccountId)
}

r.clearEnvAws, err = cmd.Flags().GetBool(commonflags.ClearEnvAWSFlag)
Expand Down Expand Up @@ -257,10 +256,11 @@ func (r *Runner) Run(ctx context.Context) error {
// only update aws provider info if user requires it.
if r.clearEnvAws && env.Properties.Providers != nil {
env.Properties.Providers.Aws = nil
} else if r.providers.Aws != nil && r.providers.Aws.Scope != nil {
} else if r.providers.Aws != nil && (r.providers.Aws.AccountID != nil && r.providers.Aws.Region != nil) {
if env.Properties.Providers == nil {
env.Properties.Providers = &corerpv20250801.Providers{}
}

env.Properties.Providers.Aws = r.providers.Aws
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/cli/cmd/env/update/preview/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ func Test_Run(t *testing.T) {
ResourceGroupName: to.Ptr("testResourceGroup"),
},
Aws: &v20250801preview.ProvidersAws{
Scope: to.Ptr("test-aws-scope"),
Region: to.Ptr("us-west-2"),
AccountID: to.Ptr("testAWSAccount"),
},
Kubernetes: &v20250801preview.ProvidersKubernetes{
Namespace: to.Ptr("test-namespace"),
Expand Down
15 changes: 15 additions & 0 deletions pkg/cli/objectformats/objectformats.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,18 @@ func GetRecipesForEnvironmentTableFormat() output.FormatterOptions {
},
}
}

func GetProvidersForEnvironmentTableFormat() output.FormatterOptions {
return output.FormatterOptions{
Columns: []output.Column{
{
Heading: "PROVIDER",
JSONPath: "{ .Provider }",
},
{
Heading: "PROPERTIES",
JSONPath: "{ .Properties }",
},
},
}
}
13 changes: 13 additions & 0 deletions pkg/cli/test_client_factory/radius_core.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,19 @@ func WithEnvironmentServerNoError() corerpfake.EnvironmentsServer {
EnvironmentResource: v20250801preview.EnvironmentResource{
Name: to.Ptr(environmentName),
Properties: &v20250801preview.EnvironmentProperties{
Providers: &v20250801preview.Providers{
Azure: &v20250801preview.ProvidersAzure{
SubscriptionID: to.Ptr("test-subscription-id"),
ResourceGroupName: to.Ptr("test-resource-group"),
},
Aws: &v20250801preview.ProvidersAws{
AccountID: to.Ptr("test-account-id"),
Region: to.Ptr("test-region"),
},
Kubernetes: &v20250801preview.ProvidersKubernetes{
Namespace: to.Ptr("test-namespace"),
},
},
RecipePacks: []*string{
to.Ptr("/planes/radius/local/resourceGroups/test-group/providers/Radius.Core/recipePacks/test-recipe-pack"),
},
Expand Down
6 changes: 4 additions & 2 deletions pkg/corerp/api/v20250801preview/environment_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ func toProvidersDataModel(providers *Providers) *datamodel.Providers_v20250801pr
// Convert AWS provider
if providers.Aws != nil {
result.AWS = &datamodel.ProvidersAWS_v20250801preview{
Scope: to.String(providers.Aws.Scope),
Region: to.String(providers.Aws.Region),
AccountID: to.String(providers.Aws.AccountID),
}
}

Expand Down Expand Up @@ -184,7 +185,8 @@ func fromProvidersDataModel(providers *datamodel.Providers_v20250801preview) *Pr
// Convert AWS provider
if providers.AWS != nil {
result.Aws = &ProvidersAws{
Scope: to.Ptr(providers.AWS.Scope),
Region: to.Ptr(providers.AWS.Region),
AccountID: to.Ptr(providers.AWS.AccountID),
}
}

Expand Down
7 changes: 5 additions & 2 deletions pkg/corerp/api/v20250801preview/zz_generated_models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 7 additions & 3 deletions pkg/corerp/api/v20250801preview/zz_generated_models_serde.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions pkg/corerp/datamodel/environment_v20250801preview.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ type ProvidersKubernetes_v20250801preview struct {

// ProvidersAWS_v20250801preview represents the AWS provider configuration.
type ProvidersAWS_v20250801preview struct {
// Scope is the target scope for AWS resources to be deployed into.
Scope string `json:"scope"`
// AccountID is the AWS account ID hosting deployed resources.
AccountID string `json:"accountId"`

// Region is the AWS region hosting deployed resources.
Region string `json:"region"`
}
2 changes: 1 addition & 1 deletion pkg/recipes/configloader/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func getConfigurationV20250801(environment *v20250801preview.EnvironmentResource
}
if envDatamodel.Properties.Providers.AWS != nil {
config.Providers.AWS = datamodel.ProvidersAWS{
Scope: envDatamodel.Properties.Providers.AWS.Scope,
Scope: "/planes/aws/aws/accounts/" + envDatamodel.Properties.Providers.AWS.AccountID + "/regions/" + envDatamodel.Properties.Providers.AWS.Region,
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using direct string concatenation with the + operator to construct the AWS scope path. For better performance and idiomatic Go code, use fmt.Sprintf or strings.Builder when building strings from multiple parts. Consider using fmt.Sprintf with a format string like "/planes/aws/aws/accounts/%s/regions/%s" for clarity.

Copilot generated this review using guidance from repository custom instructions.
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/recipes/configloader/environment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,8 @@ func TestGetConfigurationV20250801(t *testing.T) {
Properties: &modelv20250801.EnvironmentProperties{
Providers: &modelv20250801.Providers{
Aws: &modelv20250801.ProvidersAws{
Scope: to.Ptr(awsScope),
AccountID: to.Ptr("000"),
Region: to.Ptr("cool-region"),
},
Kubernetes: &modelv20250801.ProvidersKubernetes{
Namespace: to.Ptr(envNamespace),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1304,13 +1304,18 @@
"type": "object",
"description": "The AWS cloud provider definition.",
"properties": {
"scope": {
"accountId": {
"type": "string",
"description": "Target scope for AWS resources to be deployed into. For example: '/planes/aws/aws/accounts/000000000000/regions/us-west-2'."
"description": "AWS account ID for AWS resources to be deployed into."
},
"region": {
"type": "string",
"description": "AWS region for AWS resources to be deployed into."
}
},
"required": [
"scope"
"accountId",
"region"
]
},
"ProvidersAzure": {
Expand Down
7 changes: 5 additions & 2 deletions typespec/Radius.Core/environments.tsp
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,11 @@ model ProvidersKubernetes {

@doc("The AWS cloud provider definition.")
model ProvidersAws {
@doc("Target scope for AWS resources to be deployed into. For example: '/planes/aws/aws/accounts/000000000000/regions/us-west-2'.")
scope: string;
@doc("AWS account ID for AWS resources to be deployed into.")
accountId: string;

@doc("AWS region for AWS resources to be deployed into.")
region: string;
}

model Providers {
Expand Down
Loading