rad credential show - support for IRSA #7757
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7757 +/- ##
==========================================
- Coverage 61.07% 61.03% -0.04%
==========================================
Files 521 521
Lines 27229 27276 +47
==========================================
+ Hits 16630 16648 +18
- Misses 9133 9161 +28
- Partials 1466 1467 +1 ☔ View full report in Codecov by Sentry. |
nithyatsu
temporarily deployed
to
functional-tests
GitHub Actions
Inactive
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
| @@ -76,23 +76,52 @@ func Test_credentialFormat_Azure_WorkloadIdentity(t *testing.T) { | |||
| require.Equal(t, expected, buffer.String()) | |||
| } | |||
|
|
|||
| func Test_credentialFormat_AWS(t *testing.T) { | |||
| func Test_credentialFormat_AWS_AcessKey(t *testing.T) { | |||
There was a problem hiding this comment.
We should write the whole name of the function.
| func Test_credentialFormat_AWS_AcessKey(t *testing.T) { | |
| func Test_ credentialFormatAWSAccessKey(t *testing.T) { |
|
|
||
| err := output.Write(output.FormatTable, obj, buffer, credentialFormatOutput) | ||
| require.NoError(t, err) | ||
|
|
||
| expected := "NAME REGISTERED ACCESSKEYID\ntest true test-access-key-id\n" | ||
| require.Equal(t, expected, buffer.String()) | ||
| } | ||
|
|
||
| func Test_credentialFormat_AWS_IRSA(t *testing.T) { |
| } | ||
|
|
||
| type AWSAccessKeyCredentialProperties struct { | ||
| // Kind is the credential kind (Must be AccessKey) |
There was a problem hiding this comment.
| // Kind is the credential kind (Must be AccessKey) | |
| // Kind is the credential kind (must be AccessKey) |
| AccessKeyID *string | ||
| } | ||
|
|
||
| type AWSIRSACredentialProperties struct { | ||
| // Kind is the credential kind (Must be IRSA) |
There was a problem hiding this comment.
| // Kind is the credential kind (Must be IRSA) | |
| // Kind is the credential kind (must be IRSA) |
| Enabled: true, | ||
| }, | ||
| AWSCredentials: &AWSCredentialProperties{ | ||
| Kind: (*string)(awsAccessKeyCredentials.Kind), |
There was a problem hiding this comment.
Can this ever throw a nil pointer exception or something like that? Should this conversion be done beforehand to make sure there'd be no errors?
There was a problem hiding this comment.
we cannot store a credential with out kind, since the data model would not permit it and throw an error saying invalid kind. Therefore upon get, this field cannnot be null.
"code": "HttpRequestPayloadAPISpecValidationFailed",
"message": "HTTP request payload failed validation against API specification with one or more errors. Please see details for more information.",
"target": "ucp/openapi",
"details": [
{
"code": "InvalidProperties",
"message": "$.properties.kind in body should be one of [AccessKey IRSA]"
}
]
| Enabled: true, | ||
| }, | ||
| AWSCredentials: &AWSCredentialProperties{ | ||
| Kind: (*string)(awsIRSACredentials.Kind), |
There was a problem hiding this comment.
Same question here as above.
There was a problem hiding this comment.
we cannot store a credential with out kind, since the data model would not permit it and throw an error saying invalid kind. Therefore upon get, this field cannnot be null.
"code": "HttpRequestPayloadAPISpecValidationFailed",
"message": "HTTP request payload failed validation against API specification with one or more errors. Please see details for more information.",
"target": "ucp/openapi",
"details": [
{
"code": "InvalidProperties",
"message": "$.properties.kind in body should be one of [AccessKey IRSA]"
}
]
| }, | ||
| { | ||
| Heading: "ROLEARN", | ||
| JSONPath: "{ .AWSCredentials.IRSA.RoleARN }", |
There was a problem hiding this comment.
Do we also want to add HEADING: Kind ?
There was a problem hiding this comment.
I was thinking its a bit evident since the column name changes between rolearn and accesskey. but looks like azure credentials have a kind column. I will implement this. cc @Reshrahim @willtsai
nithyatsu
temporarily deployed
to
functional-tests
GitHub Actions
Inactive
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
nithyatsu
force-pushed
the
irsa_show
branch
2 times, most recently
from
5d96335 to
2f23e92
Compare
nithyatsu
temporarily deployed
to
functional-tests
GitHub Actions
Inactive
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Signed-off-by: nithyatsu <[email protected]>
Signed-off-by: nithyatsu <[email protected]>
nithyatsu
temporarily deployed
to
functional-tests
GitHub Actions
Inactive
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
# Description rad credential show should work with the new datamodel that supports 2 aws credential types - accesskey and irsa ## Type of change - This pull request adds or changes features of Radius and has an approved issue (issue link required). Partially Fixes: radius-project#7618 --------- Signed-off-by: nithyatsu <[email protected]> Signed-off-by: Reshma Abdul Rahim <[email protected]>
Description
rad credential show should work with the new datamodel that supports 2 aws credential types - accesskey and irsa
Type of change
Partially Fixes: Add IRSA (workload identity) support for AWS cloud provider #7618