feat: add crn parser submodule #2

MatthewLemmond · 2024-08-31T00:20:24Z

Description

Add the crn parser submodule and initial module cleanup from template

Release required?

No release
Patch release (x.x.X)
Minor release (x.X.x)
Major release (X.x.x)

Release notes content

Add CRN parser submodule

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.

MatthewLemmond · 2024-08-31T00:20:30Z

/run pipeline

…-common-utilities into crn-parser

MatthewLemmond · 2024-09-03T14:59:54Z

/run pipeline

ocofaigh

Left some comments.
Also can you address these:

remove the root level tf files
we dont need an upgrade test do we? we are not creating any resources in the crn parser module
since there is no root level module, I would move the Usage, Required IAM access policies, and Requirements section to the individual subnmodule readme. And then delete the terraform-ibm-common-utilities heading

ocofaigh · 2024-09-04T15:58:40Z

README.md

@@ -20,16 +20,19 @@ For information, see "Module names and descriptions" at
 https://terraform-ibm-modules.github.io/documentation/#/implementation-guidelines?id=module-names-and-descriptions
 -->

-TODO: Replace this with a description of the modules in this repo.
+This module serves as a repository to hold common Terraform utilities used in other modules in the terraform-ibm-modules organization.


"This module" seems wrong wording here. There is no root level module. It should probably start with "This repository serves as a ..."

examples/crn-parser/outputs.tf

ocofaigh · 2024-09-04T16:02:35Z

modules/crn-parser/version.tf

+terraform {
+  required_version = ">= 1.3.0"
+  # If your module requires any terraform providers, uncomment the "required_providers" section below and add all required providers.
+  # Each required provider's version should be a flexible range to future proof the module's usage with upcoming minor and patch versions.


I think remove the code comments

SKIP UPGRADE TEST because we moved the examples from the basic/advanced to one example for the submodule upgrade test fails

MatthewLemmond · 2024-09-05T15:11:01Z

/run pipeline

ocofaigh

final few comments

ocofaigh · 2024-09-06T11:37:53Z

README.md

@@ -20,46 +20,22 @@ For information, see "Module names and descriptions" at
 https://terraform-ibm-modules.github.io/documentation/#/implementation-guidelines?id=module-names-and-descriptions
 -->

-TODO: Replace this with a description of the modules in this repo.
+This repository serves as a repository to hold common Terraform utilities used in other modules in the terraform-ibm-modules organization.


"This repository serves as a library" maybe reads batter? - cc @SirSpidey

ocofaigh · 2024-09-06T11:38:12Z

.github/CODEOWNERS

@@ -1,2 +1,2 @@
 # Primary owner should be listed first in list of global owners, followed by any secondary owners
-*       @SirSpidey @ocofaigh
+*       @MatthewLemmond


add a secondary owner

ocofaigh · 2024-09-06T11:42:19Z

examples/crn-parser/main.tf

+  region            = var.region
+  key_protect_name  = "${var.prefix}-kp"
+  access_tags       = var.resource_tags
+}


We really don't need to provision key protect instance here, or a resource group. It kind of doesn't make sense as a real use case, because that module already supports outputting region, guid etc, so you wouldn't use the CRN parser here.
I suggest that the example just takes in a CRN as a single input variable. In the test we can then pass it a mock CRN value. (it means updating the test to use TestOptionsDefault instead of TestOptionsDefaultWithVars)

SirSpidey

some style suggestions

SirSpidey · 2024-09-06T14:40:00Z

README.md

@@ -20,46 +20,22 @@ For information, see "Module names and descriptions" at
 https://terraform-ibm-modules.github.io/documentation/#/implementation-guidelines?id=module-names-and-descriptions
 -->

-TODO: Replace this with a description of the modules in this repo.
+This repository serves as a repository to hold common Terraform utilities used in other modules in the terraform-ibm-modules organization.


Suggested change

This repository serves as a repository to hold common Terraform utilities used in other modules in the terraform-ibm-modules organization.

Common Terraform utilities that are used in other modules in the terraform-ibm-modules GitHub organization.

I think you can shorten to get to the core info

SirSpidey · 2024-09-06T14:46:58Z

README.md


+Each utility is placed under the [modules](./modules) directory so they may be individually referenced within a Terraform project, see the [Overview](#overview) section below for a list of available submodules.


Suggested change

Each utility is placed under the [modules](./modules) directory so they may be individually referenced within a Terraform project, see the [Overview](#overview) section below for a list of available submodules.

You can reference any utility in this repo from your Terraform project by pointing to the utility in the [modules](/modules) directory. For a list of available submodules, see the Submodules in the [Overview](#overview) section.

SirSpidey · 2024-09-06T14:48:43Z

examples/crn-parser/README.md

+An example that will provision the following:
+
+- A new resource group if one is not passed in.
+- A new Key Protect instance.
+
+The output of the example will be the CRN of the Key Protect instance and the parsed fields of the CRN using the CRN parser submodule.


Suggested change

An example that will provision the following:

- A new resource group if one is not passed in.

- A new Key Protect instance.

The output of the example will be the CRN of the Key Protect instance and the parsed fields of the CRN using the CRN parser submodule.

This example provisions the following resources:

- A new resource group, if one is not passed in.

- A new Key Protect instance.

The outputs are the CRN of the Key Protect instance and the fields of the CRN that are parsed by using the CRN parser utility.

Using utility instead of submodule to stay consistent with the main readme file.

But it looks like this won't need the KP instance, based on Conall's comment. So readme could have something instead like,

This example takes a CRN as the input. The outputs are the fields of the CRN that are parsed by using the CRN parser utility.

@MatthewLemmond You should use segments instead of fields for the parts of the CRN to stay internally consistent and consistent with the cloud docs.

SirSpidey · 2024-09-06T14:54:28Z

examples/crn-parser/version.tf

+terraform {
+  required_version = ">= 1.3.0"
+
+  # As the CRN parser does not utilize the IBM Terraform provider, the provider does not need to be locked to any version


Not critical, but I'd replace As with Because to make it easier for non-native English speakers to understand. And replace utilize with use.

Suggested change

# As the CRN parser does not utilize the IBM Terraform provider, the provider does not need to be locked to any version

# Because the CRN parser doesn't use the IBM Terraform provider, the provider doesn't need to be locked to a version.

No longer necessary, version.tf is now just the required terraform version and dropped the need for the IBM provider.

SirSpidey · 2024-09-06T14:57:39Z

modules/crn-parser/README.md

@@ -0,0 +1,51 @@
+# Terraform IBM common utilities CRN parser
+
+This module takes a CRN string input and returns each of the fields in the provided CRN as a separate output, for more information on what each of these fields represent see <https://cloud.ibm.com/docs/account?topic=account-crn>.


Suggested change

This module takes a CRN string input and returns each of the fields in the provided CRN as a separate output, for more information on what each of these fields represent see <https://cloud.ibm.com/docs/account?topic=account-crn>.

This module takes a CRN string input, parses the CRN, and returns the segments in the CRN as separate output fields. For more information about what each CRN segment represents, see [Cloud Resource Names](https://cloud.ibm.com/docs/account?topic=account-crn).

SirSpidey · 2024-09-06T14:58:29Z

modules/crn-parser/variables.tf

-#}
+variable "crn" {
+  type        = string
+  description = "The CRN to be parsed."


Suggested change

description = "The CRN to be parsed."

description = "The CRN to parse."

MatthewLemmond · 2024-09-06T15:45:58Z

/run pipeline

MatthewLemmond · 2024-09-06T17:15:25Z

/run pipeline

SirSpidey

one really small update. otherwise 👍

SirSpidey · 2024-09-06T17:21:08Z

examples/crn-parser/README.md

- A new Key Protect instance.
-
-The output of the example will be the CRN of the Key Protect instance and the parsed fields of the CRN using the CRN parser submodule.
+This example takes a CRN as the input. The outputs are the fields of the CRN that are parsed by using the CRN parser utility.


Suggested change

This example takes a CRN as the input. The outputs are the fields of the CRN that are parsed by using the CRN parser utility.

This example takes a CRN as the input. The outputs are the segments of the CRN that are parsed by using the CRN parser utility.

just want to keep this consistent with modules/crn-parser/README.md

MatthewLemmond · 2024-09-06T18:04:59Z

/run pipeline

MatthewLemmond · 2024-09-06T18:27:55Z

@ocofaigh one thing I thought about while fixing a test failure, do we need the CRA scan since this is just a local? I presume any utilities we'd add in the future would be similar in that they don't need the provider so might be better to just pull it from the module

MatthewLemmond · 2024-09-09T12:59:16Z

/run pipeline

…-common-utilities into crn-parser

MatthewLemmond · 2024-09-09T13:25:03Z

/run pipeline

terraform-ibm-modules-ops · 2024-09-11T08:23:58Z

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

feat: add crn parser submodule

8e61400

MatthewLemmond requested review from SirSpidey and ocofaigh as code owners August 31, 2024 00:20

MatthewLemmond added 2 commits September 3, 2024 10:59

Merge branch 'main' of github.com:terraform-ibm-modules/terraform-ibm…

5fa681d

…-common-utilities into crn-parser

refactor: fix cra target

1afbb6a

ocofaigh requested changes Sep 4, 2024

View reviewed changes

MatthewLemmond added 2 commits September 5, 2024 10:38

refactor: skipping upgrade test

b061030

SKIP UPGRADE TEST because we moved the examples from the basic/advanced to one example for the submodule upgrade test fails

refactor: pr feedback

74796b3

ocofaigh requested changes Sep 6, 2024

View reviewed changes

SirSpidey suggested changes Sep 6, 2024

View reviewed changes

refactor: cleanup

855fa10

MatthewLemmond requested review from SirSpidey and ocofaigh September 6, 2024 15:46

refactor: fix cra config

4086caa

SirSpidey suggested changes Sep 6, 2024

View reviewed changes

refactor: missed a segment

4444d6c

MatthewLemmond requested a review from SirSpidey September 6, 2024 18:06

SirSpidey approved these changes Sep 6, 2024

View reviewed changes

Merge branch 'main' into crn-parser

2747518

Merge branch 'main' of github.com:terraform-ibm-modules/terraform-ibm…

c10e131

…-common-utilities into crn-parser

ocofaigh approved these changes Sep 11, 2024

View reviewed changes

ocofaigh merged commit 116534d into main Sep 11, 2024
2 checks passed

ocofaigh deleted the crn-parser branch September 11, 2024 08:23

terraform-ibm-modules-ops added the released label Sep 11, 2024

	This repository serves as a repository to hold common Terraform utilities used in other modules in the terraform-ibm-modules organization.
	Common Terraform utilities that are used in other modules in the terraform-ibm-modules GitHub organization.


		Each utility is placed under the [modules](./modules) directory so they may be individually referenced within a Terraform project, see the [Overview](#overview) section below for a list of available submodules.

	Each utility is placed under the [modules](./modules) directory so they may be individually referenced within a Terraform project, see the [Overview](#overview) section below for a list of available submodules.
	You can reference any utility in this repo from your Terraform project by pointing to the utility in the [modules](/modules) directory. For a list of available submodules, see the Submodules in the [Overview](#overview) section.

	# As the CRN parser does not utilize the IBM Terraform provider, the provider does not need to be locked to any version
	# Because the CRN parser doesn't use the IBM Terraform provider, the provider doesn't need to be locked to a version.

		@@ -0,0 +1,51 @@
		# Terraform IBM common utilities CRN parser

		This module takes a CRN string input and returns each of the fields in the provided CRN as a separate output, for more information on what each of these fields represent see <https://cloud.ibm.com/docs/account?topic=account-crn>.

	This module takes a CRN string input and returns each of the fields in the provided CRN as a separate output, for more information on what each of these fields represent see <https://cloud.ibm.com/docs/account?topic=account-crn>.
	This module takes a CRN string input, parses the CRN, and returns the segments in the CRN as separate output fields. For more information about what each CRN segment represents, see [Cloud Resource Names](https://cloud.ibm.com/docs/account?topic=account-crn).

	description = "The CRN to be parsed."
	description = "The CRN to parse."

	This example takes a CRN as the input. The outputs are the fields of the CRN that are parsed by using the CRN parser utility.
	This example takes a CRN as the input. The outputs are the segments of the CRN that are parsed by using the CRN parser utility.

feat: add crn parser submodule #2

feat: add crn parser submodule #2

Uh oh!

Conversation

MatthewLemmond commented Aug 31, 2024

Description

Release required?

Release notes content

Run the pipeline

Checklist for reviewers

For mergers

Uh oh!

MatthewLemmond commented Aug 31, 2024

Uh oh!

MatthewLemmond commented Sep 3, 2024

Uh oh!

ocofaigh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatthewLemmond commented Sep 5, 2024

Uh oh!

ocofaigh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SirSpidey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatthewLemmond commented Sep 6, 2024

Uh oh!

MatthewLemmond commented Sep 6, 2024

Uh oh!

SirSpidey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatthewLemmond commented Sep 6, 2024

Uh oh!

MatthewLemmond commented Sep 6, 2024

Uh oh!

MatthewLemmond commented Sep 9, 2024

Uh oh!

MatthewLemmond commented Sep 9, 2024

Uh oh!

Uh oh!

terraform-ibm-modules-ops commented Sep 11, 2024

Uh oh!

Uh oh!

ocofaigh left a comment •

edited

Loading