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

merge multiple PR missing in Release branch #773

Open
wants to merge 27 commits into
base: release-0.6
Choose a base branch
from

Conversation

sushanth0910
Copy link
Contributor

What this PR does / why we need it:
This PR contains changes from following PR's which were merged into Master branch but not pulled into Release branch


#630
What this PR does / why we need it:

currently authenticator only returns 401 and it is hard to know if the request is really unauthorized or it is due to aws sts throttle.
This PR will create a new metric in authenticator to record the STS throttling count.
This PR will return 429 to api-server when authenticator got STS throttling from AWS STS service


#741
What this PR does / why we need it:

This behavior and test is prep for an AWS SDK update, to ensure that the generated token (signature) matches when we update the SDK.

What this PR does / why we need it:

Preparation for #736


#750
What this PR does / why we need it:

This simplifies the API, and removes the unnecessary GetWithRoleForSession() method. This also simplifies migration to aws-sdk-go-v2 by allowing both Generator (mostly) and TokenOptions to be not bound to a specific SDK version.

Which issue(s) this PR fixes

Preparation for #736 migration


#753

What this PR does / why we need it:

While working on #736, I noticed that the current authenticator is setting x-amz-expires header to 60 nanoseconds instead of 60 seconds.

Which issue(s) this PR fixes (optional, in fixes #(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

This value is ignored by STS, but this changes the signed header value to not be 0, but now be 60 seconds.


#681

What this PR does / why we need it:

Replace deprecated ioutil package; this was deprecated back in go 1.16 

golang/go#42026


#755

What this PR does / why we need it:

The token filecache used to use a private global function for creating a filelock, and overrode it in tests with a hand-crafted mocks for filesystem and environment variable operations.

This change adds adds injectability to the filecache's filesystem and file lock using afero. This change also will simplify future changes when updating the AWS SDK with new credential interfaces.


#756

What this PR does / why we need it:
Update filecache to use AWS SDK Go V2 with wrappers

This changes updates filecache's internal types to use the AWS SDK Go v2's types, while preserving the external interface used by /pkg/token.This will simplify the future project-wide change for AWS SDK Go v2.

Depends on #755 to be merged first

Which issue(s) this PR fixes

First change toward #736


#759

What this PR does / why we need it:
I encountered this bug:

❯ go mod tidy
go: downloading k8s.io/code-generator v0.31.0
go: downloading gopkg.in/evanphx/json-patch.v4 v4.12.0
go: downloading github.com/emicklei/go-restful/v3 v3.11.1
verifying github.com/emicklei/go-restful/[email protected]: checksum mismatch
downloaded: h1:PZ8lyY5omynnq9Y2BHKmNqkC3AnPMES2e2bg7slcfzw=
go.sum: h1:S+9bSbua1z3FgCnV0KKOSSZ3mDthb5NyEPL5gEpCvyk=

SECURITY ERROR
This download does NOT match an earlier download recorded in go.sum.
The bits may have been replaced on the origin server, or an attacker may
have intercepted the download attempt.

For more information, see 'go help module-auth'.

Reported in the go-restful repo here:
emicklei/go-restful#542

The PR is the result of running:
go get github.com/emicklei/go-restful/[email protected]
and committing the changes.


#769
What this PR does / why we need it:
This would set the http timeout for the requests made to sts inorder to fail fast during an issue. By default the request can be blocked forever if connection is established or timeout after 30 sec if its fails to establish a connection.


#768
What this PR does / why we need it:

addresses vulnerabilities fixed in newer go versions


#767

What this PR does / why we need it:
This PR adds few logs to find if the Verify token presigned URL is sent to Global/Regional end point.
This PR also adds new metrics dimention to STS metrics to get requests failed/throttled on regional/global endpoint.


#770

What this PR does / why we need it:
Adding kmala to the owners list

contributions made: https://github.com/kubernetes-sigs/aws-iam-authenticator/pulls?q=is%3Apr+author%3Akmala

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

nnmin-aws and others added 26 commits November 13, 2024 04:01
This behavior and test is prep for an AWS SDK update, to ensure that the
generated token (signature) matches when we update the SDK.

Signed-off-by: Micah Hausler <[email protected]>
This simplifies the API, and removes the unnecessary `GetWithRoleForSession()`,
`GetWithRole()`, and `Get()` methods. This also simplifies migration to
aws-sdk-go-v2 by allowing both Generator and TokenOptions to be not bound to a
specific SDK version.
The token filecache used to use a private global function for creating a
filelock, and overrode it in tests with a hand-crafted mocks for
filesystem and environment variable operations.

This change adds adds injectability to the filecache's filesystem and
file lock using afero. This change also will simplify future changes
when updating the AWS SDK with new credential interfaces.

Signed-off-by: Micah Hausler <[email protected]>
This changes updates filecache's internal types to use the AWS SDK Go
v2's types, while preserving the external interface used by /pkg/token.
This will simplify the future project-wide change for AWS SDK Go v2.

Signed-off-by: Micah Hausler <[email protected]>
Signed-off-by: Luke Swart <[email protected]>
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 13, 2024
@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 13, 2024
@k8s-ci-robot
Copy link
Contributor

Hi @sushanth0910. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 13, 2024
@kmala
Copy link
Contributor

kmala commented Nov 13, 2024

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 13, 2024
@kmala
Copy link
Contributor

kmala commented Nov 13, 2024

/test pull-aws-iam-authenticator-unit

@kmala
Copy link
Contributor

kmala commented Nov 13, 2024

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 13, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kmala, sushanth0910
Once this PR has been reviewed and has the lgtm label, please assign nnmin-aws for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants