Migration to use terraform cross-object referencing for input variable and refactor the code #245
Conversation
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
iamar7
changed the title
|
/run pipeline |
|
/run pipeline |
There was a problem hiding this comment.
There seems to be some issues in your validation logic. Can you please check and test them? Can you also review the other variables - I would of expected to see more validation logic, especially since we have some many different combinations and permutations.
|
|
||
| validation { | ||
| condition = !(var.existing_cos_kms_key_crn != null && !var.skip_cos_kms_auth_policy && var.existing_kms_instance_crn == null) | ||
| error_message = "The existing_kms_instance_crn is not provided and is required to configure the COS - KMS authorization policy." |
There was a problem hiding this comment.
It is not only required to configure the auth policy. Its also required to create a new key if user is not using an existing one
There was a problem hiding this comment.
I'm not sure this validation is working as expected. I pulled it locally and terraform plan passed with all the default values:
existing_kms_instance_crn = nullexisting_cos_kms_key_crn = nullskip_cos_kms_auth_policy = false
Also the validation doesn't seem to be taking into consideration the case where user is proving their own buckets. In that case, there is no KMS details needed right?
There was a problem hiding this comment.
When I was looking at the condition here, I found that it throws error only in one case which is when
existing_cos_kms_key_crnis not null & we setskip_cos_kms_key_auth_policyas false,- now if a user doesn't provide
existing_kms_instance_crnit throws an error which technically is wrong since
we don't need an input for existing_kms_instance_crn when we are providing existing_cos_kms_key_crn.
Further investigating as to why we have done this....
Description
Resolves: #182
Release required?
x.x.X)x.X.x)X.x.x)Release notes content
Run the pipeline
If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.
Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:
Checklist for reviewers
For mergers