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

Added Injected Identity Source #337

Conversation

bradkwadsworth-mw
Copy link

Signed-off-by: Brad Wadsworth brad.wadsworth@mavenwave.com

Description of your changes

Add the ability to use InjectedIdentity as a CommonCredentialExtractor option. This will just return a nil value and will rely on the specific provider to take the appropriate action in order to fetch the injected credentials.

Fixes #335

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Tested with workload identity using a forked helm provider in a google GKE cluster.

Signed-off-by: Brad Wadsworth <brad.wadsworth@mavenwave.com>
@djalanxyz
Copy link

Related code section:
./apis/common/v1/resource.go:236:
CredentialsSourceInjectedIdentity CredentialsSource = "InjectedIdentity"

@@ -95,6 +95,8 @@ func CommonCredentialExtractor(ctx context.Context, source xpv1.CredentialsSourc
return ExtractFs(ctx, afero.NewOsFs(), selector)
case xpv1.CredentialsSourceSecret:
return ExtractSecret(ctx, client, selector)
case xpv1.CredentialsSourceInjectedIdentity:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given we don't have to extract any credentials, in this case, we can check whether the source equals xpv1.CredentialsSourceInjectedIdentity before calling CommonCredentialExtractor.
That approach makes it more explicit regarding the intent.

See this as an example.

@bradkwadsworth-mw
Copy link
Author

Decided to only modify provider for InjectedIdentity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants