feat(circleci-oidc): Add support for the provider and the role #501
Conversation
simonweil force-pushed the feature/add-circleci-oidc-support branch from e924b64 to 45d3b35 Compare | @antonbabenko why does CI fail on |
| @antonbabenko ping... |
| @bryantbiggs @antonbabenko any chance to get a go or no go? |
|
If we decide that we need nullable, we need to update |
| variable "org_uuid" { | ||
| description = "The CircleCI organization UUID that will be authorized to assume the role." | ||
| type = string | ||
| nullable = false |
There was a problem hiding this comment.
When create = false, the variables without default values must still be set since nullable is set to false. Right?
Please add examples for this module similar to https://github.com/terraform-aws-modules/terraform-aws-iam/blob/master/examples/iam-github-oidc/
There was a problem hiding this comment.
Yes. Thinking about it, what is the point in the create variable?
Will add the example.
| @@ -0,0 +1,55 @@ | |||
| locals { | |||
| # Just extra safety incase someone passes in a url with `https://` | |||
There was a problem hiding this comment.
| # Just extra safety incase someone passes in a url with `https://` | |
| # Just extra safety in case someone passes in a url with `https://` |
| description = "Policies to attach to the IAM role in `{'static_name' = 'policy_arn'}` format" | ||
| type = map(string) | ||
| default = {} | ||
| nullable = false |
There was a problem hiding this comment.
Nullables are not needed in most places in the module. At least, I think so. We don't have them in pretty much any modules.
@bryantbiggs What do you think?
There was a problem hiding this comment.
IMHO, it is very useful to set nullable = false anywhere you do not expect to get nulls so you do not need to handle situations when null is set and there is an expected default. Also it is useful in sub modules for handling defaults by setting null in the input to make that default of the variable take place.
We do it for all our modules :)
But, your call, I can remove if you think it is better for the community.
There was a problem hiding this comment.
I don't see the value in using nullable - we haven't seen a use case yet where the resolution was to use nullable
| in general - I don't like the idea of having a module per CI/CD provider. I know in the past I have pushed back when someone wanted to add a new module for Bamboo CI. If we went forward, I would rather we create a module that is generic, and lets users opt into the different 3rd party provider (default) functionality. Something like |
| I agree that we don't want to have a module per CI/CD provider, so let's update this PR to bring generic In the longer run, we may discontinue the one we already have for GitHub. |
Sounds good to me, I'll sketch up a direction for discussion |
| This PR has been automatically marked as stale because it has been open 30 days |
| Don't close, I'm planning to get to it |
| This PR has been automatically marked as stale because it has been open 30 days |
| This PR was automatically closed because of stale in 10 days |
| I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
3 participants
Description
Created a CircleCI OIDC provider and role
Motivation and Context
It's hard to do it on your own and I'm happy to contribute it to the community
Breaking Changes
None
How Has This Been Tested?
examples/*to demonstrate and validate my change(s)examples/*projectspre-commit run -aon my pull requestCloses: #500