implement irsa server side support #7738
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7738 +/- ##
==========================================
- Coverage 61.11% 61.08% -0.03%
==========================================
Files 520 521 +1
Lines 27190 27229 +39
==========================================
+ Hits 16618 16634 +16
- Misses 9104 9131 +27
+ Partials 1468 1464 -4 ☔ 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... |
nithyatsu
force-pushed
the
ucp_irsa
branch
4 times, most recently
from
2fb90ae to
3db7d16
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... |
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
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
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... |
pkg/ucp/aws/ucpcredentialprovider.go
Outdated
|
|
||
| // CredentialKind is IRSA | ||
| CredentialKindIRSA = "IRSA" | ||
| // CredentialKindAccessKey is AccessKey |
There was a problem hiding this comment.
| // CredentialKindAccessKey is AccessKey | |
| // CredentialKind is AccessKey |
| }) | ||
|
|
||
| t.Run("valid credential", func(t *testing.T) { | ||
| p := newMockProvider() | ||
| t.Run("valid redential", func(t *testing.T) { |
pkg/ucp/credentials/aws.go
Outdated
| storage = c | ||
| default: | ||
| return nil, errors.New("invalid AWSAccessKeyCredentialProperties") | ||
| } |
There was a problem hiding this comment.
is it possible to combine both these?
eg. case *ucpapi.AwsAccessKeyCredentialProperties, *ucpapi.AwsIRSACredentialProperties
There was a problem hiding this comment.
nice one. But it does not like it when I tried, since in the next line we have switch c := p.Storage.(type) and looks since now p can correspond to either of 2 values, it gives an error.
There was a problem hiding this comment.
let's pull common code into function and each switch case calls the function, eg:
func getStorageProperties(p any) (*InternalCredentialStorageProperties, error) {
switch c := p.(type) {
case *InternalCredentialStorageProperties:
return c, nil
default:
return nil, errors.New("invalid AWS credential storage properties")
}
}
| // Radius requests will first be routed to STS endpoint, | ||
| // where it will be validated and then the request to the specific service (such as S3) will be made using | ||
| // the bearer token from the STS response. | ||
| // Based on the https://docs.aws.amazon.com/IAM/latest/UserGuide/id_credentials_temp_enable-regions.html, | ||
| // STS endpoint should be region based, and in the same region as | ||
| // Radius instance to minimize latency associated with STS call and thereby improve performance. |
There was a problem hiding this comment.
This comment seems out of place because the code below isn't making any calls to AWS, was this intended?
There was a problem hiding this comment.
I have added "once we switch to region based STS endpoint, we should add the region to the config." to the comment. Hope that makes it clearer. issues #7747 is at a high level going to introduce a way to set region for both ucp and terraform, based on radius's location.
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
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 <nithyasu@microsoft.com>
nithyatsu
temporarily deployed
to
functional-tests
GitHub Actions
Inactive
kachawla
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... |
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 Add server side support for AWS IRSA. UCP handles AWS resource deployment and needs irsa suuport. Terraform provider communicates with AWS directly and needs IRSA support too. ## 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 <nithyasu@microsoft.com> Signed-off-by: Nithya Subramanian <98416062+nithyatsu@users.noreply.github.com> Co-authored-by: Karishma Chawla <kachawla@microsoft.com> Signed-off-by: Reshma Abdul Rahim <reshmarahim.abdul@microsoft.com>
Description
Add server side support for AWS IRSA.
UCP handles AWS resource deployment and needs irsa suuport.
Terraform provider communicates with AWS directly and needs IRSA support too.
Type of change
Partially Fixes: #7618