adding api_oidc_config support

[kpatron-cockroachlabs]

This change adds a new API OIDC Config resource type.

Commit checklist

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

---

This change is 

[kpatron-cockroachlabs]

This requires a release of go sdk and for conflicts with the newer go-sdk to be resolved before this can merge

[erademacher]

Reviewed 6 of 17 files at r1, all commit messages.
Reviewable status: 6 of 17 files reviewed, 5 unresolved discussions (waiting on @jenngeorge and @kpatron-cockroachlabs)

---

internal/provider/api_oidc_config.go line 47 at r1 (raw file):

					stringplanmodifier.UseStateForUnknown(),
				},
				MarkdownDescription: "ID of the API OIDC Configuration. Required by Terraform.",

Is this an actual UUID that a user would have on hand for terraform import? If it is, the "Required by Terraform" wording isn't really necessary. If this is a synthetic ID, please describe how to build it so users can import their existing configs.

---

internal/provider/api_oidc_config.go line 63 at r1 (raw file):

			"claim": schema.StringAttribute{
				Required:    true,
				Description: "The JWT claim that should be used as the user identifier. Defaults to the subject if an empty string is provided.",

Does this actually need to be required? Could the user omit it instead of passing in an empty string?

---

internal/provider/api_oidc_config.go line 67 at r1 (raw file):

			"identity_map": schema.StringAttribute{
				Required:    true,
				Description: "The mapping rules to convert token user identifiers into a new form.",

I wasn't able to find anything on the map syntax here through a google search. Do values for this field follow some sort of standard OIDC syntax? If so, it'd be helpful to link to a doc or something. If this is our proprietary syntax, it would definitely be good to break this out into an object with all the map components in a separate field.

---

internal/provider/api_oidc_config.go line 150 at r1 (raw file):

			resp.Diagnostics.AddWarning(
				"API OIDC Config not found",
				"API OIDC Config not found. Maintenance window will be removed from state.")

s/Maintenance window/API OIDC Config

---

internal/provider/api_oidc_config_test.go line 35 at r1 (raw file):

// TestIntegrationApiOIdcConfigResource attempts to create, check, and destroy
// an API OIDC Config, but uses a mocked API service.
func TestIntegrationApiOIdcConfigResource(t *testing.T) {

Could you also add an acceptance test shell with t.Skip() and a comment about why it can't be implemented?

[kpatron-cockroachlabs]

Reviewable status: 6 of 17 files reviewed, 5 unresolved discussions (waiting on @erademacher and @jenngeorge)

---

internal/provider/api_oidc_config.go line 47 at r1 (raw file):

Previously, erademacher (Evan Rademacher) wrote…

Is this an actual UUID that a user would have on hand for terraform import? If it is, the "Required by Terraform" wording isn't really necessary. If this is a synthetic ID, please describe how to build it so users can import their existing configs.

This is a UUID the backend creates. I just copied the wording from somewhere else. Happy to delete that bit.

---

internal/provider/api_oidc_config.go line 63 at r1 (raw file):

Previously, erademacher (Evan Rademacher) wrote…

Does this actually need to be required? Could the user omit it instead of passing in an empty string?

I was struggling with our API. Basically you can omit it just fine but it confuses TF because when you get the field from the API it returns an empty string which is "different" from being omitted. I tried making it optional with a default value of empty string but the references I found to set a default empty string didn't work here for some reason (stringdefault was missing if I remember correctly).

---

internal/provider/api_oidc_config.go line 67 at r1 (raw file):

Previously, erademacher (Evan Rademacher) wrote…

I wasn't able to find anything on the map syntax here through a google search. Do values for this field follow some sort of standard OIDC syntax? If so, it'd be helpful to link to a doc or something. If this is our proprietary syntax, it would definitely be good to break this out into an object with all the map components in a separate field.

It's https://www.postgresql.org/docs/current/auth-username-maps.html without the first column. We unfortunately, don't document our use of this well from CRDB which is why I didn't link to any docs yet.

---

internal/provider/api_oidc_config.go line 150 at r1 (raw file):

Previously, erademacher (Evan Rademacher) wrote…

s/Maintenance window/API OIDC Config

Will fix.

[jenngeorge]

Reviewable status: 5 of 17 files reviewed, 9 unresolved discussions (waiting on @erademacher and @kpatron-cockroachlabs)

---

examples/resources/cockroach_api_oidc_config/cockroach_api_oidc_config.tf line 4 at r2 (raw file):

  issuer       = "https://accounts.google.com"
  audience     = "test_audience"
  jwks         = "{\"keys\":[{\"alg\":\"RS256\",\"e\":\"AQAB\",\"kid\":\"test_kid1\",\"kty\":\"RSA\",\"n\":\"09lq1lCEuteonwDJOhGTDak11ThplZuC9JEWQNdBnBSQwlkJQIE7A7nTBO0xTibcsh2HwYkC-N_Gs1jP4iwN3dRqnu5FwG2ct5mY8KLwJiHzToFC0MKenSFQCy0FviNtOnpiObcUlDvR2NDeNtMl_6SPzcQEt7GUTBBYZgoAxPmOgevki6ZNO6Y86xFqx3y6v8EPwW010AiC60r4AHGCTBhYF4uqmq5JH2UU4dDh9Udc-9LZxlSqPwJvnKDG2GjcnD8TsU3wjfEM_nRmx3dnXsrZUXYfNGtdv5dlHywf5AhkJmTavqcsJkgrNA-PNBghFMcCR816_kCIkCYWLWC5vQ\"}]}"

I prefer using jsonencode for JSON string values because (imo) it's easier to type and read.
Here is an example from the aws provider: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy.html

---

internal/provider/api_oidc_config.go line 64 at r2 (raw file):

				Optional:    true,
				Computed:    true,
				Description: "The JWT claim that should be used as the user identifier. Defaults to the subject if an empty string is provided.",

nit: What do you think about updating the second sentence of the description to just "Defaults to the subject."?

---

internal/provider/api_oidc_config_test.go line 36 at r2 (raw file):

// a real API OIDC Config. It will be skipped if TF_ACC isn't set.
// In order to work the ApiOidcEnabled Feature Flag must be enabled and
// the test org must have Org SSO enabled (no need for SAML/OIDC).

@erademacher is it ok for us to turn on SSO and that feature flag for the test org?

---

internal/provider/api_oidc_config_test.go line 50 at r2 (raw file):

// TestIntegrationApiOIdcConfigResource attempts to create, check, and destroy
// an API OIDC Config, but uses a mocked API service.
func TestIntegrationApiOIdcConfigResource(t *testing.T) {

supernit: The I in OIdc is uppercase for TestAccApiOIdcConfigResource & TestIntegrationApiOIdcConfigResource but lowercase elsewhere.

[kpatron-cockroachlabs]

Reviewable status: 5 of 17 files reviewed, 8 unresolved discussions (waiting on @erademacher and @jenngeorge)

---

examples/resources/cockroach_api_oidc_config/cockroach_api_oidc_config.tf line 4 at r2 (raw file):

Previously, jenngeorge (Jenn Georgevich) wrote…

I prefer using jsonencode for JSON string values because (imo) it's easier to type and read.
Here is an example from the aws provider: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy.html

It's an interesting tradeoff. I agree that would make it easier to read but the usual method users will have for getting the JWKS is by pulling it from a website like https://www.googleapis.com/oauth2/v3/certs. In those cases it might be easier for the author to just copy and past rather than converting it to TF format.

---

internal/provider/api_oidc_config.go line 64 at r2 (raw file):

Previously, jenngeorge (Jenn Georgevich) wrote…

nit: What do you think about updating the second sentence of the description to just "Defaults to the subject."?

Done.

---

internal/provider/api_oidc_config_test.go line 50 at r2 (raw file):

Previously, jenngeorge (Jenn Georgevich) wrote…

supernit: The I in OIdc is uppercase for TestAccApiOIdcConfigResource & TestIntegrationApiOIdcConfigResource but lowercase elsewhere.

Good catch.

[erademacher]

Sorry for the delay. Looks great so far. Almost there. Just need to make a final decision on the structure of identity_map and rerun codegen.

Reviewable status: 5 of 17 files reviewed, 5 unresolved discussions (waiting on @jenngeorge and @kpatron-cockroachlabs)

---

internal/provider/api_oidc_config.go line 47 at r1 (raw file):

Previously, kpatron-cockroachlabs (Kyle) wrote…

This is a UUID the backend creates. I just copied the wording from somewhere else. Happy to delete that bit.

Thanks. Don't forget to rerun make generate.

---

internal/provider/api_oidc_config.go line 67 at r1 (raw file):

Previously, kpatron-cockroachlabs (Kyle) wrote…

It's https://www.postgresql.org/docs/current/auth-username-maps.html without the first column. We unfortunately, don't document our use of this well from CRDB which is why I didn't link to any docs yet.

I guess my real question is "how do we make this as easy to use as possible?" Is this likely something users are going to pass in as a reference to a field from another resource? Or paste it in from an external source? If so, string is great. If it's something the user is probably going to need to populate manually, or get all three components from separate resources, then I think you should separate this out into its own object with map_name, system_username, and database_username fields.

If you still feel like a string is the best way to go, please add a link to that Postgres doc in the MarkdownDescription field.

---

internal/provider/api_oidc_config_test.go line 36 at r2 (raw file):

Previously, jenngeorge (Jenn Georgevich) wrote…

@erademacher is it ok for us to turn on SSO and that feature flag for the test org?

Yeah, go for it. This org is only used for these tests, so we have free reign over feature flags.

[jenngeorge]

Reviewable status: 5 of 17 files reviewed, 4 unresolved discussions (waiting on @erademacher and @kpatron-cockroachlabs)

---

examples/resources/cockroach_api_oidc_config/cockroach_api_oidc_config.tf line 4 at r2 (raw file):

Previously, kpatron-cockroachlabs (Kyle) wrote…

It's an interesting tradeoff. I agree that would make it easier to read but the usual method users will have for getting the JWKS is by pulling it from a website like https://www.googleapis.com/oauth2/v3/certs. In those cases it might be easier for the author to just copy and past rather than converting it to TF format.

Ok thanks, the copy&pasted string sgtm!

---

internal/provider/api_oidc_config_test.go line 36 at r2 (raw file):

Previously, erademacher (Evan Rademacher) wrote…

Yeah, go for it. This org is only used for these tests, so we have free reign over feature flags.

Thanks, done! Org SSO is enabled (org members received a re-invite email) and ApiOidcEnabled is enabled.

[jenngeorge]

Thanks for the identity_map structure change! after the one nit.

Reviewed 8 of 18 files at r4.
Reviewable status: 8 of 19 files reviewed, 3 unresolved discussions (waiting on @erademacher and @kpatron-cockroachlabs)

---

internal/provider/api_oidc_config.go line 71 at r4 (raw file):

						"token_identity": schema.StringAttribute{
							Required:    true,
							Description: "The token value that needs to be mapped",

nit: Period at end of description (then rerun make generate to update api_oidc_config.md ).

stale