provider: Add jwt_issuer resource #231
Conversation
pritesh-lahoti
force-pushed
the
tf-jwt-issuers
branch
from
aa98626 to
6df2b9c
Compare
pritesh-lahoti
force-pushed
the
tf-jwt-issuers
branch
from
6df2b9c to
c9359c5
Compare
pritesh-lahoti
force-pushed
the
tf-jwt-issuers
branch
4 times, most recently
from
a343c75 to
a0742bf
Compare
There was a problem hiding this comment.
I left some questions and nits in there but mostly looks good. @mdlinville Can you look over the docs here?
internal/provider/jwt_issuers.go
Outdated
| Optional: true, | ||
| Computed: true, | ||
| Description: "Indicates that the token_principal field is a regex value.", | ||
| Description: "Specifies how to map the fetched token identity to an identity in CockroachDB Cloud. In case of a regular expression for token_identity, this must contain a \\1 placeholder for the matched content.", |
There was a problem hiding this comment.
I wonder if there will be a problem understanding if the slashes in the placeholder token should be escaped or not? I noticed that in the docs they show up as \1 but in the actual source they need to use \\1. I'm not sure the best way to document this. Maybe @mdlinville can comment?
There was a problem hiding this comment.
I agree.
@mdlinville did you have a recommendation here?
I'll keep the description here and within the API in sync.
There was a problem hiding this comment.
I have no idea about this. If you need a second backslash to show up, don't you just escape it? I'm not familiar the nuts and bolts here.
There was a problem hiding this comment.
The question is how best to document what value the user should use. The current output will read as this:
...for token_identity, this must contain a \1 placeholder for the matched content.
But since the value in the string will need to be escaped, the user will actually use \\1. I was suggesting that maybe we should show \\1 instead. But asking you if you had guidance on how to make this distinction more clear to the user.
One potential way would be to link them to an example.
There was a problem hiding this comment.
Updated the field description to call this out. Open to suggestions.
pritesh-lahoti
force-pushed
the
tf-jwt-issuers
branch
from
a0742bf to
8032aef
Compare
Aligning to the CC API changes, this PR deprecates the `api_oidc_config` resource in favor of the `jwt_issuer` resource. As the existing TF resource was not in use, the team decided to make the changes in-place. Refined and added exhaustive acceptance tests. Removed is_regex field from the TF Provider.
pritesh-lahoti
force-pushed
the
tf-jwt-issuers
branch
from
8032aef to
f33fb15
Compare
|
I'll be going ahead and merging the change to avoid any further merge conflicts. |
Aligning to the CC API changes, this PR deprecates the
api_oidc_configresource in favor of thejwt_issuerresource.As the existing TF resource was not in use, the team decided to make the changes in-place.
Refined and added exhaustive acceptance tests.
Commit checklist
make generate)