add skipping variable #644

Aayush-Abhyarthi · 2023-11-29T07:36:53Z

Description

Release required?

No release
Patch release (x.x.X)
Minor release (x.X.x)
Major release (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:

/run pipeline

Checklist for reviewers

If relevant, a test for the change is included or updated with this PR.
If relevant, documentation for the change is included or updated with this PR.

For mergers

Use a conventional commit message to set the release level. Follow the guidelines.
Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
Use the Squash and merge option.

variables.tf

ocofaigh

@Aayush-Abhyarthi I think we both agree on the renaming as per https://github.com/terraform-ibm-modules/terraform-ibm-landing-zone/pull/644/files#r1414474051

Bare in mind, the new variable will need to be exposed in all of the patterns

ocofaigh

see latest comments

ocofaigh · 2023-12-06T12:59:25Z

variables.tf

@@ -1497,10 +1497,16 @@ variable "vpc_placement_groups" {
 # s2s variables
 ##############################################################################

-variable "add_kms_block_storage_s2s" {
+variable "skip_kms_block_storage_s2s_auth_policy" {
  description = "Whether to create a service-to-service authorization between block storage and the key management service."


since the variable has changed, the description needs to be updated to: "Whether to skip the creation of a service-to-service authorization policy between block storage and the key management service."

You will need to update the description for this variable everywhere it exists too

ocofaigh · 2023-12-06T13:01:08Z

variables.tf

+}
+
+variable "skip_all_s2s_auth_policies" {
+  description = "Set it to true to create the authorization policy."


"Whether to skip the creation of all of the service-to-service authorization policies. If setting to true, policies must be in place on the account before provisioning."

You will need to update the description for this variable everywhere you added it too

patterns/vsi/variables.tf

patterns/vsi/module/variables.tf

patterns/vpc/variables.tf

patterns/vpc/module/variables.tf

patterns/roks/variables.tf

patterns/roks/module/variables.tf

patterns/mixed/variables.tf

ocofaigh · 2023-12-06T13:05:42Z

dynamic_values/config_modules/service_authorizations/service_authorizations.tf

@@ -43,7 +43,7 @@ locals {
 module "kms_to_block_storage" {
  source = "../list_to_map"
  list = [
-    for instance in(var.add_kms_block_storage_s2s ? ["block-storage"] : []) :
+    for instance in(var.skip_kms_block_storage_s2s_auth_policy ? ["block-storage"] : []) :


The logic needs to be reversed now since skip_kms_block_storage_s2s_auth_policy will have value of false now

ocofaigh · 2023-12-07T12:04:39Z

@Aayush-Abhyarthi A PR was merged yesterday that has a reference to the add_kms_block_storage_s2s variable in /patterns/vsi-extension/README.md - please update that as part of this PR too since we are changing the variable name. Thanks

ocofaigh · 2023-12-12T17:11:17Z

@Aayush-Abhyarthi Can you resolve conflicts? Is there anything remaining in this PR? If PR is ready for review, can you remove the [DO NOT MERGE] text from the title? Thanks

Aayush-Abhyarthi · 2023-12-13T07:49:27Z

/run pipeline

ocofaigh · 2023-12-14T15:41:26Z

/run pipeline

ocofaigh · 2023-12-14T15:42:32Z

@toddgiguere any further concerns with this PR? I'm thinking it might be good now?

ocofaigh · 2023-12-15T11:33:25Z

/run pipeline

ocofaigh · 2023-12-15T14:22:39Z

patterns/mixed/config.tf

+    key_management                         = lookup(local.override[local.override_type], "key_management", local.config.key_management)
+    atracker                               = lookup(local.override[local.override_type], "atracker", local.config.atracker)
+    clusters                               = lookup(local.override[local.override_type], "clusters", local.config.clusters)
+    wait_till                              = lookup(local.override[local.override_type], "wait_till", "IngressReady")


A change was made on this line recently and look like you do not have it in your branch, so hence TFLint is failing. Can you replace "IngressReady" with var.wait_till

ocofaigh · 2023-12-15T14:23:03Z

patterns/roks/module/config.tf

+    key_management                         = lookup(local.override[local.override_type], "key_management", local.config.key_management)
+    atracker                               = lookup(local.override[local.override_type], "atracker", local.config.atracker)
+    clusters                               = lookup(local.override[local.override_type], "clusters", local.config.clusters)
+    wait_till                              = lookup(local.override[local.override_type], "wait_till", "IngressReady")


same as https://github.com/terraform-ibm-modules/terraform-ibm-landing-zone/pull/644/files#r1428029441

ocofaigh · 2023-12-15T14:23:47Z

patterns/vpc/module/config.tf

+    key_management                         = lookup(local.override[local.override_type], "key_management", local.config.key_management)
+    atracker                               = lookup(local.override[local.override_type], "atracker", local.config.atracker)
+    clusters                               = lookup(local.override[local.override_type], "clusters", local.config.clusters)
+    wait_till                              = lookup(local.override[local.override_type], "wait_till", "IngressReady")


same as https://github.com/terraform-ibm-modules/terraform-ibm-landing-zone/pull/644/files#r1428029441

patterns/vsi/module/config.tf

patterns/roks/module/config.tf

ocofaigh

@Aayush-Abhyarthi spotted another change needed

Aayush-Abhyarthi · 2023-12-18T08:54:10Z

/run pipeline

ocofaigh · 2023-12-18T09:32:34Z

@Aayush-Abhyarthi You must have introduced a bug somewhere..

│ Error: Inconsistent conditional result types
│ 
│   on ../../service_authorizations.tf line 19, in resource "ibm_iam_authorization_policy" "policy":
│   19:   for_each                    = var.skip_all_s2s_auth_policies == true ? [] : local.authorization_policies
│     ├────────────────
│     │ local.authorization_policies is object with 6 attributes
│     │ var.skip_all_s2s_auth_policies is false
│ 
│ The true and false result expressions must have consistent types. The
│ 'true' value is tuple, but the 'false' value is object.
``

Aayush-Abhyarthi · 2023-12-18T10:21:09Z

/run pipeline

Aayush-Abhyarthi · 2024-01-06T22:51:29Z

/run pipeline

Aayush-Abhyarthi · 2024-01-08T09:46:43Z

/run pipeline

ocofaigh · 2024-01-08T11:43:18Z

Will run the pipeline once #670 is merged (since this PR will need a rebase then anyway)

ocofaigh · 2024-01-08T13:01:16Z

@Aayush-Abhyarthi looks like there is now a conflict in this PR after rebase. Can you resolve it please. Hold off on running the pipeline though, as #677 is running now and will be merged first

Aayush-Abhyarthi · 2024-01-08T22:31:51Z

/run pipeline

ocofaigh · 2024-01-09T15:09:38Z

/run pipeline

ocofaigh · 2024-01-10T10:14:19Z

The TestRunVSIQuickStartPatternSchematics test failed with Error: cannot add more than 20 gateways - Let me do some cleanup and re-run.

ocofaigh · 2024-01-10T10:23:43Z

@Aayush-Abhyarthi There was another PR ready so I just merged it and now this has conflicts again. Can you please resolve, and I'll priortise this PR once conflicts addressed. Thanks

Aayush-Abhyarthi · 2024-01-10T11:12:20Z

/run pipeline

terraform-ibm-modules-ops · 2024-01-10T14:20:59Z

🎉 This PR is included in version 5.11.0 🎉

The release is available on:

GitHub release
v5.11.0

Your semantic-release bot 📦🚀

add skipping variable

cbac92a

ocofaigh reviewed Dec 4, 2023

View reviewed changes

variables.tf Show resolved Hide resolved

ocofaigh requested changes Dec 5, 2023

View reviewed changes

Aayush-Abhyarthi and others added 2 commits December 6, 2023 00:13

renaming policy skipping variables

b329d00

Merge branch 'main' into skip-s2s-auth-policy-creation

1229e5c

ocofaigh requested changes Dec 6, 2023

View reviewed changes

Merge branch 'main' into skip-s2s-auth-policy-creation

b62f2f1

Aayush-Abhyarthi and others added 2 commits December 7, 2023 21:04

Merge branch 'main' into skip-s2s-auth-policy-creation

ed8c45d

update descriptions

4c820a9

resolve conflicts

3eda99c

Aayush-Abhyarthi changed the title ~~[DO NOT MERGE]add skipping variable~~ add skipping variable Dec 13, 2023

Merge branch 'main' into skip-s2s-auth-policy-creation

7e45d2e

ocofaigh requested a review from toddgiguere December 14, 2023 15:42

toddgiguere previously approved these changes Dec 14, 2023

View reviewed changes

Merge branch 'main' into skip-s2s-auth-policy-creation

31f4a22

ocofaigh reviewed Dec 15, 2023

View reviewed changes

patterns/vsi/module/config.tf Show resolved Hide resolved

ocofaigh reviewed Dec 15, 2023

View reviewed changes

patterns/roks/module/config.tf Show resolved Hide resolved

ocofaigh requested changes Dec 15, 2023

View reviewed changes

Aayush-Abhyarthi and others added 2 commits December 15, 2023 20:18

Merge branch 'main' into skip-s2s-auth-policy-creation

1df1aff

add wait_till

8e9cfe4

Aayush-Abhyarthi dismissed toddgiguere’s stale review via 8e9cfe4 December 15, 2023 15:05

Merge branch 'main' into skip-s2s-auth-policy-creation

d6b1e4f

fix: inconsistent conditional types

640d04d

Aayush-Abhyarthi added 2 commits December 20, 2023 17:36

fix inconsistent conditional types

c108818

fix: resolve conflicts

a44c41a

update common dev assets

10cb446

Merge branch 'main' into skip-s2s-auth-policy-creation

8d4e3e3

Aayush-Abhyarthi added 2 commits January 9, 2024 03:58

fix: resolve conflicts

32ce82a

Merge branch 'main' into skip-s2s-auth-policy-creation

3a231ec

Merge branch 'main' into skip-s2s-auth-policy-creation

9b60488

ocofaigh mentioned this pull request Jan 9, 2024

fix: use service_endpoints input variable #663

Merged

6 tasks

ocofaigh previously approved these changes Jan 9, 2024

View reviewed changes

fix: resolve conflicts

6126eab

Aayush-Abhyarthi dismissed ocofaigh’s stale review via 6126eab January 10, 2024 11:11

ocofaigh approved these changes Jan 10, 2024

View reviewed changes

ocofaigh merged commit 7e68518 into main Jan 10, 2024
2 checks passed

ocofaigh deleted the skip-s2s-auth-policy-creation branch January 10, 2024 14:19

terraform-ibm-modules-ops added the released label Jan 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add skipping variable #644

add skipping variable #644

Aayush-Abhyarthi commented Nov 29, 2023 •

edited

Loading

ocofaigh left a comment

ocofaigh left a comment

ocofaigh Dec 6, 2023

ocofaigh Dec 6, 2023

ocofaigh Dec 6, 2023

ocofaigh Dec 6, 2023

ocofaigh Dec 6, 2023

ocofaigh commented Dec 7, 2023

ocofaigh commented Dec 12, 2023

Aayush-Abhyarthi commented Dec 13, 2023

ocofaigh commented Dec 14, 2023

ocofaigh commented Dec 14, 2023

ocofaigh commented Dec 15, 2023

ocofaigh Dec 15, 2023

ocofaigh Dec 15, 2023

ocofaigh Dec 15, 2023

ocofaigh left a comment

Aayush-Abhyarthi commented Dec 18, 2023

ocofaigh commented Dec 18, 2023

Aayush-Abhyarthi commented Dec 18, 2023

Aayush-Abhyarthi commented Jan 6, 2024

Aayush-Abhyarthi commented Jan 8, 2024

ocofaigh commented Jan 8, 2024 •

edited

Loading

ocofaigh commented Jan 8, 2024

Aayush-Abhyarthi commented Jan 8, 2024

ocofaigh commented Jan 9, 2024

ocofaigh commented Jan 10, 2024

ocofaigh commented Jan 10, 2024

Aayush-Abhyarthi commented Jan 10, 2024

terraform-ibm-modules-ops commented Jan 10, 2024

add skipping variable #644

add skipping variable #644

Conversation

Aayush-Abhyarthi commented Nov 29, 2023 • edited Loading

Description

Release required?

Release notes content

Run the pipeline

Checklist for reviewers

For mergers

ocofaigh left a comment

Choose a reason for hiding this comment

ocofaigh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocofaigh commented Dec 7, 2023

ocofaigh commented Dec 12, 2023

Aayush-Abhyarthi commented Dec 13, 2023

ocofaigh commented Dec 14, 2023

ocofaigh commented Dec 14, 2023

ocofaigh commented Dec 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocofaigh left a comment

Choose a reason for hiding this comment

Aayush-Abhyarthi commented Dec 18, 2023

ocofaigh commented Dec 18, 2023

Aayush-Abhyarthi commented Dec 18, 2023

Aayush-Abhyarthi commented Jan 6, 2024

Aayush-Abhyarthi commented Jan 8, 2024

ocofaigh commented Jan 8, 2024 • edited Loading

ocofaigh commented Jan 8, 2024

Aayush-Abhyarthi commented Jan 8, 2024

ocofaigh commented Jan 9, 2024

ocofaigh commented Jan 10, 2024

ocofaigh commented Jan 10, 2024

Aayush-Abhyarthi commented Jan 10, 2024

terraform-ibm-modules-ops commented Jan 10, 2024

Aayush-Abhyarthi commented Nov 29, 2023 •

edited

Loading

ocofaigh commented Jan 8, 2024 •

edited

Loading