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

[CC-30333] auth: use JWT for authentication #249

Merged
merged 1 commit into from
Nov 15, 2024
Merged

[CC-30333] auth: use JWT for authentication #249

merged 1 commit into from
Nov 15, 2024

Conversation

pritesh-lahoti
Copy link
Contributor

@pritesh-lahoti pritesh-lahoti commented Nov 15, 2024

This PR allows using the Terraform Provider via JWT authentication, in addition to API Keys. The JWT auth mechanism requires a CC_VANITY_NAME env capturing the vanity name of the org with the corresponding JWT Issuer. In case the JWT is issued against multiple identities, it also requires a CC_USERNAME env capturing the user / service account to impersonate. Eventually, we will add a CI stage for running acceptance tests via this auth mechanism.

Commit checklist

  • Changelog
  • Doc gen (make generate)
  • Integration test(s)
  • Acceptance test(s)
  • Example(s)

internal/provider/provider.go Outdated Show resolved Hide resolved
internal/provider/provider.go Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
Copy link

@kathancox kathancox left a comment

Choose a reason for hiding this comment

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

lgtm

@pritesh-lahoti
Copy link
Contributor Author

lgtm

@kathancox, requesting another round of review, as there have been further changes to the docs. Thank you!

@fantapop
Copy link
Collaborator

This one seems a bit off, but lets fix the message in another round. We can fix later. It's talking about both JWT Issuer and Invalid API key.

    jwt_issuers_test.go:148: Step 1/3 error: Error running apply: exit status 1

        Error: Error creating JWT Issuer

          with cockroach_jwt_issuer.test,
          on terraform_plugin_test.tf line 12, in resource "cockroach_jwt_issuer" "test":
          12: resource "cockroach_jwt_issuer" "test" {

        Could not create JWT Issuer: invalid API key
--- FAIL: TestAccJWTIssuerResource (0.34s)

Copy link
Collaborator

@fantapop fantapop left a comment

Choose a reason for hiding this comment

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

this looks great. Thank you. I mentioned one nit but we can wait to the next version to fix.

@fantapop
Copy link
Collaborator

@pritesh-lahoti i just noticed the commit message needs to be updated with the new env var names.

Copy link

@kathancox kathancox left a comment

Choose a reason for hiding this comment

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

Two nits, but LGTM!

docs/index.md Outdated
@@ -20,8 +20,9 @@ provider "cockroach" {
# Instructions for getting an API Key
# https://www.cockroachlabs.com/docs/cockroachcloud/console-access-management.html#api-access
#
# The Terraform provider requires an environment variable COCKROACH_API_KEY
# The Terraform provider requires either COCKROACH_API_KEY or COCKROACH_API_JWT environment variable for performing authentication.

Choose a reason for hiding this comment

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

Suggested change
# The Terraform provider requires either COCKROACH_API_KEY or COCKROACH_API_JWT environment variable for performing authentication.
# The Terraform provider requires either the COCKROACH_API_KEY or COCKROACH_API_JWT environment variable for performing authentication.

@@ -5,6 +5,7 @@ provider "cockroach" {
# Instructions for getting an API Key
# https://www.cockroachlabs.com/docs/cockroachcloud/console-access-management.html#api-access
#
# The Terraform provider requires an environment variable COCKROACH_API_KEY
# The Terraform provider requires either COCKROACH_API_KEY or COCKROACH_API_JWT environment variable for performing authentication.

Choose a reason for hiding this comment

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

Suggested change
# The Terraform provider requires either COCKROACH_API_KEY or COCKROACH_API_JWT environment variable for performing authentication.
# The Terraform provider requires either the COCKROACH_API_KEY or COCKROACH_API_JWT environment variable for performing authentication.

This PR allows using the Terraform Provider via JWT authentication, in
addition to API Keys. The JWT auth mechanism requires a
COCKROACH_VANITY_NAME env var capturing the vanity name of the org with
the corresponding JWT Issuer.  In case the JWT is issued against
multiple identities, it also requires a COCKROACH_USERNAME env var
capturing the user / service account to impersonate.  Eventually, we
will add a CI stage for running acceptance tests via this auth
mechanism.
@fantapop
Copy link
Collaborator

I'm going to go ahead and make these small nit changes and land the PR. There is still time to get the release out.

@fantapop fantapop merged commit f149712 into main Nov 15, 2024
4 checks passed
@fantapop fantapop deleted the jwt-auth branch November 15, 2024 21:27
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.

4 participants