Identity provider aws iam auth e2e tests #556
Conversation
abhay-krishna
added
approved
size/XL
pokearu
force-pushed
the
identity-provider-aws-iam-auth-e2e
branch
from
c3bfa4e to
2941720
Compare
internal/pkg/awsiam/kubectl.go
Outdated
|
|
||
| // GetAllPods uses the kubectl installed on the host machine | ||
| // Modifies the command context to add ./bin to path | ||
| func GetAllPods(ctx context.Context, opts ...string) ([]corev1.Pod, error) { |
There was a problem hiding this comment.
Don't need to change it, but couldn't we do this using the kubectl in executables?
we would just need to implement a new Executable that runs the magic to add ./bin to the path
It seems more flexible and we don't duplicate code (plus the get pods method in executables already has unit tests
There was a problem hiding this comment.
So the main reason i rewrote this was because the kubectl in executables uses the cli tools docker image.
When i am running kubectl here I pass in the kubeconfig for aws-iam-auth which has an exec to use the aws-iam-authenticator client binary which is downloaded in to ./bin
When running kubectl through docker run the container does not have the aws-iam-auth client in it.
I am open to changing this, but I couldnt get an easy solution for using the downloaded local binary with docker run?
There was a problem hiding this comment.
The kubectl in executables doesn't always use the cli-tools image, it depends on the dependency we inject
For example, in the cli, it's possible to disable the cli-tools image and use directly the binaries from the host, it's just another type implementing the Executable interface
I'm proposing doing something similar, implementing that interface in a way that allows you to add the iam-authenticator to the path
pokearu
force-pushed
the
identity-provider-aws-iam-auth-e2e
branch
4 times, most recently
from
07a15ae to
1830ada
Compare
test/framework/awsiam.go
Outdated
| if err != nil { | ||
| return fmt.Errorf("error finding current working directory: %v", err) | ||
| } | ||
| err = os.Setenv("PATH", fmt.Sprintf("%s/bin:%s", workDir, envPath)) |
There was a problem hiding this comment.
nit: no big deal but you could check first if this has already been added to the path before adding it
To prevent adding it twice
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: g-gaston, pokearu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
pokearu
force-pushed
the
identity-provider-aws-iam-auth-e2e
branch
from
1830ada to
981d3d9
Compare
|
/lgtm |
* Adding framework support for aws-iam-authenticator tests * Refactored gzip file download * Adding e2e tests for aws-iam-authenticator * Enabling feature gates activation from e2e run command * Refactoring changes based on framework updates, reverting e2e --gates flag * Refactor IamAuth test validations to use existing kubectl * Checking env PATH before updating
* Adding framework support for aws-iam-authenticator tests * Refactored gzip file download * Adding e2e tests for aws-iam-authenticator * Enabling feature gates activation from e2e run command * Refactoring changes based on framework updates, reverting e2e --gates flag * Refactor IamAuth test validations to use existing kubectl * Checking env PATH before updating
Issue #90
Description of changes:
The changes include e2e tests for aws-iam-authenticator as an identity provider.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.