"rad credential register aws" command changes for irsa #7750

vishwahiremat · 2024-07-19T22:26:48Z

Description

Updating the existing rad credential register aws command:

rad credential register aws --access-key-id <access-key-id> --secret-access-key <secret-access-key> should be replaced with rad credential register aws access-key --access-key-id <access-key-id> --secret-access-key <secret-access-key>

rad credential register aws irsa --iam-role <roleARN> should be supported.

Type of change

This pull request fixes a bug in Radius and has an approved issue (issue link required).
This pull request adds or changes features of Radius and has an approved issue (issue link required).
This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional).

Fixes: #issue_number

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

codecov · 2024-07-19T22:38:58Z

Codecov Report

Attention: Patch coverage is 71.06918% with 46 lines in your changes missing coverage. Please review.

Project coverage is 61.05%. Comparing base (81b89fd) to head (283a68f).

Files	Patch %	Lines
...cmd/credential/register/aws/accesskey/accesskey.go	81.08%	7 Missing and 7 partials ⚠️
pkg/cli/cmd/credential/register/aws/irsa/irsa.go	81.53%	6 Missing and 6 partials ⚠️
pkg/cli/cmd/credential/register/aws/aws.go	0.00%	11 Missing ⚠️
pkg/cli/cmd/credential/register/register.go	0.00%	5 Missing ⚠️
pkg/cli/cmd/credential/credential.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7750      +/-   ##
==========================================
- Coverage   61.06%   61.05%   -0.01%     
==========================================
  Files         521      523       +2     
  Lines       27276    27361      +85     
==========================================
+ Hits        16655    16706      +51     
- Misses       9157     9184      +27     
- Partials     1464     1471       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

radius-functional-tests · 2024-07-22T20:20:24Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	vishwahiremat/radius
Commit ref	`500ac59`
Unique ID	funca2b58f3e0e
Image tag	pr-funca2b58f3e0e

Click here to see the list of tools in the current test run

gotestsum 1.12.0
KinD: v0.20.0
Dapr: 1.12.0
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.1.0
Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-funca2b58f3e0e
Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-funca2b58f3e0e
controller test image location: ghcr.io/radius-project/dev/controller:pr-funca2b58f3e0e
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-funca2b58f3e0e
deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting corerp-cloud functional tests...
❌ Failed to install Radius for corerp-cloud functional test. Please check the logs for more details
⌛ Starting ucp-cloud functional tests...
❌ corerp-cloud functional test failed. Please check the logs for more details
⌛ Starting datastoresrp-cloud functional tests...
❌ ucp-cloud functional test cancelled. Please check the logs for more details
❌ datastoresrp-cloud functional test cancelled. Please check the logs for more details

pkg/cli/cmd/credential/register/aws/aws.go

nithyatsu · 2024-07-23T22:08:23Z

pkg/cli/cmd/credential/register/aws/accesskey/accesskey.go

+		},
+	}
+
+	err = client.PutAWS(ctx, credential)


if we have access key credential configured, and run rad credential register, then we would just overwrite it, since we store exactly one credential which is "default" correct?

nithyatsu

LGTM. would be great if @Reshrahim / @willtsai review the ux messages and are ok with them.

ytimocin · 2024-07-24T15:52:24Z

.github/workflows/functional-test.yaml

Needs a rebase with main because this workflow doesn't exist anymore. The name has changed.

pkg/cli/cmd/credential/register/aws/accesskey/accesskey.go

pkg/cli/cmd/credential/register/register.go

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

Reshrahim · 2024-07-24T19:39:29Z

pkg/cli/cmd/credential/credential.go

-rad credential register aws --access-key-id <access-key-id> --secret-access-key <secret-access-key>
+# Register (Add or update) cloud provider credential for AWS with access key authentication.
+rad credential register aws access-key --access-key-id <access-key-id> --secret-access-key <secret-access-key>
+# Register (Add or update) cloud provider credential for AWS with IRSA.


Suggested change

# Register (Add or update) cloud provider credential for AWS with IRSA.

# Register (Add or update) cloud provider credential for AWS with IRSA (IAM Roles for Service Accounts).

Abbreviating IRSA wherever possible in docs would be helpful for the user

pkg/cli/cmd/credential/register/register.go

Reshrahim · 2024-07-24T19:42:24Z

LGTM. would be great if @Reshrahim / @willtsai review the ux messages and are ok with them.

Just took a look and suggested abbreviation for IRSA in couple of places.

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

radius-functional-tests · 2024-07-26T17:10:33Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	vishwahiremat/radius
Commit ref	`6c60fb1`
Unique ID	func7611f87dfa
Image tag	pr-func7611f87dfa

Click here to see the list of tools in the current test run

gotestsum 1.12.0
KinD: v0.20.0
Dapr: 1.12.0
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.1.0
Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-func7611f87dfa
Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-func7611f87dfa
controller test image location: ghcr.io/radius-project/dev/controller:pr-func7611f87dfa
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-func7611f87dfa
deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting ucp-cloud functional tests...
⌛ Starting datastoresrp-cloud functional tests...
⌛ Starting corerp-cloud functional tests...
❌ Failed to install Radius for ucp-cloud functional test. Please check the logs for more details
❌ ucp-cloud functional test failed. Please check the logs for more details
❌ Failed to install Radius for datastoresrp-cloud functional test. Please check the logs for more details
❌ datastoresrp-cloud functional test failed. Please check the logs for more details
❌ corerp-cloud functional test cancelled. Please check the logs for more details

ytimocin · 2024-07-26T19:27:01Z

pkg/cli/cmd/credential/register/aws/accesskey/accesskey.go

+		return clierrors.Message("Access Key id %q cannot be empty.", r.AccessKeyID)
+	}
+	if r.SecretAccessKey == "" {
+		return clierrors.Message("Secret Access Key %q cannot be empty.", r.SecretAccessKey)


Why do we need to print the empty AccessKey and AccessKeyID here? I mean why do we need %q here?

this is the exisitng code for access-key type. we moved it to different file

ytimocin · 2024-07-26T19:27:40Z

pkg/cli/cmd/credential/register/aws/accesskey/accesskey.go

+	if err != nil {
+		return err
+	}
+	credential := ucp.AwsCredentialResource{


nit: extra line before this

ytimocin · 2024-07-26T19:28:20Z

pkg/cli/cmd/credential/register/aws/accesskey/accesskey_test.go

Do we need to add any other tests here?

we havent changed anything for the access-key type. the tests were copied from the previously added tests.

ytimocin · 2024-07-26T19:30:14Z

pkg/cli/cmd/credential/register/aws/irsa/irsa.go

+Radius will use the provided IAM credential for all interactions with AWS. 
+` + common.LongDescriptionBlurb,
+		Example: `
+# Register (Add or update) cloud provider credential for AWS with IAM authentication


We have the same information for both IRSA and Access Key. Is that right?

updadted this documentation for ira kind

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

radius-functional-tests · 2024-07-26T20:43:30Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	vishwahiremat/radius
Commit ref	`2d4dcc9`
Unique ID	funcd31c13758a
Image tag	pr-funcd31c13758a

Click here to see the list of tools in the current test run

gotestsum 1.12.0
KinD: v0.20.0
Dapr: 1.12.0
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.1.0
Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-funcd31c13758a
Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-funcd31c13758a
controller test image location: ghcr.io/radius-project/dev/controller:pr-funcd31c13758a
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-funcd31c13758a
deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting datastoresrp-cloud functional tests...
⌛ Starting ucp-cloud functional tests...
⌛ Starting corerp-cloud functional tests...
❌ Failed to install Radius for datastoresrp-cloud functional test. Please check the logs for more details
❌ datastoresrp-cloud functional test failed. Please check the logs for more details
❌ corerp-cloud functional test cancelled. Please check the logs for more details
❌ ucp-cloud functional test cancelled. Please check the logs for more details

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

radius-functional-tests · 2024-07-30T16:49:00Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	vishwahiremat/radius
Commit ref	`283a68f`
Unique ID	func973a8ee04f
Image tag	pr-func973a8ee04f

Click here to see the list of tools in the current test run

gotestsum 1.12.0
KinD: v0.20.0
Dapr: 1.12.0
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.1.0
Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-func973a8ee04f
Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-func973a8ee04f
controller test image location: ghcr.io/radius-project/dev/controller:pr-func973a8ee04f
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-func973a8ee04f
deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting ucp-cloud functional tests...
⌛ Starting datastoresrp-cloud functional tests...
⌛ Starting corerp-cloud functional tests...
❌ Failed to install Radius for datastoresrp-cloud functional test. Please check the logs for more details
❌ Failed to install Radius for ucp-cloud functional test. Please check the logs for more details
❌ datastoresrp-cloud functional test failed. Please check the logs for more details
❌ ucp-cloud functional test failed. Please check the logs for more details
❌ corerp-cloud functional test cancelled. Please check the logs for more details

rynowak

…t#7750) # Description Updating the existing `rad credential register aws command`: `rad credential register aws --access-key-id <access-key-id> --secret-access-key <secret-access-key>` should be replaced with `rad credential register aws access-key --access-key-id <access-key-id> --secret-access-key <secret-access-key>` `rad credential register aws irsa --iam-role <roleARN>` should be supported. ## Type of change  - This pull request fixes a bug in Radius and has an approved issue (issue link required). - This pull request adds or changes features of Radius and has an approved issue (issue link required). - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional).  Fixes: #issue_number --------- Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com> Signed-off-by: Reshma Abdul Rahim <reshmarahim.abdul@microsoft.com>

rad credential register command changes for irsa

500ac59

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

vishwahiremat requested review from a team as code owners July 19, 2024 22:26

vishwahiremat temporarily deployed to functional-tests July 22, 2024 20:19 — with GitHub Actions Inactive

nithyatsu reviewed Jul 23, 2024

View reviewed changes

pkg/cli/cmd/credential/register/aws/aws.go Show resolved Hide resolved

nithyatsu reviewed Jul 23, 2024

View reviewed changes

nithyatsu approved these changes Jul 23, 2024

View reviewed changes

ytimocin requested changes Jul 24, 2024

View reviewed changes

Merge branch 'main' into vishwahiremat/rad-credential-update-irsa

0a0b08e

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

Reshrahim reviewed Jul 24, 2024

View reviewed changes

pkg/cli/cmd/credential/register/register.go Outdated Show resolved Hide resolved

addressing comments

6c60fb1

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

vishwahiremat temporarily deployed to functional-tests July 26, 2024 17:09 — with GitHub Actions Inactive

ytimocin reviewed Jul 26, 2024

View reviewed changes

addressing comments

2d4dcc9

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

ytimocin approved these changes Jul 26, 2024

View reviewed changes

vishwahiremat temporarily deployed to functional-tests July 26, 2024 20:42 — with GitHub Actions Inactive

Merge branch 'main' into vishwahiremat/rad-credential-update-irsa

283a68f

Signed-off-by: Vishwanath Hiremath <vhiremath@microsoft.com>

vishwahiremat temporarily deployed to functional-tests July 30, 2024 16:48 — with GitHub Actions Inactive

ytimocin merged commit 9b564db into radius-project:main Jul 30, 2024
25 of 26 checks passed

rynowak reviewed Jul 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

"rad credential register aws" command changes for irsa #7750

"rad credential register aws" command changes for irsa #7750

vishwahiremat commented Jul 19, 2024

codecov bot commented Jul 19, 2024 •

edited

Loading

radius-functional-tests bot commented Jul 22, 2024 •

edited

Loading

nithyatsu Jul 23, 2024

nithyatsu left a comment

ytimocin Jul 24, 2024

Reshrahim Jul 24, 2024

Reshrahim commented Jul 24, 2024

radius-functional-tests bot commented Jul 26, 2024 •

edited

Loading

ytimocin Jul 26, 2024

vishwahiremat Jul 26, 2024

ytimocin Jul 26, 2024

ytimocin Jul 26, 2024

vishwahiremat Jul 26, 2024

ytimocin Jul 26, 2024

vishwahiremat Jul 26, 2024

radius-functional-tests bot commented Jul 26, 2024 •

edited

Loading

radius-functional-tests bot commented Jul 30, 2024 •

edited

Loading

rynowak left a comment

	# Register (Add or update) cloud provider credential for AWS with IRSA.
	# Register (Add or update) cloud provider credential for AWS with IRSA (IAM Roles for Service Accounts).

"rad credential register aws" command changes for irsa #7750

"rad credential register aws" command changes for irsa #7750

Conversation

vishwahiremat commented Jul 19, 2024

Description

Type of change

codecov bot commented Jul 19, 2024 • edited Loading

Codecov Report

radius-functional-tests bot commented Jul 22, 2024 • edited Loading

Radius functional test overview

Test Status

Choose a reason for hiding this comment

nithyatsu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Reshrahim commented Jul 24, 2024

radius-functional-tests bot commented Jul 26, 2024 • edited Loading

Radius functional test overview

Test Status

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radius-functional-tests bot commented Jul 26, 2024 • edited Loading

Radius functional test overview

Test Status

radius-functional-tests bot commented Jul 30, 2024 • edited Loading

Radius functional test overview

Test Status

rynowak left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 19, 2024 •

edited

Loading

radius-functional-tests bot commented Jul 22, 2024 •

edited

Loading

radius-functional-tests bot commented Jul 26, 2024 •

edited

Loading

radius-functional-tests bot commented Jul 26, 2024 •

edited

Loading

radius-functional-tests bot commented Jul 30, 2024 •

edited

Loading