App config Gaps fixed - Config aggregator module added

[RiadhJouini]

Description

Fixed Gaps and added the service aggregator sub-module.

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.

[ocofaigh]

• Can you please add a README.md file in the new submodule folder. As a reference, see: https://github.com/terraform-ibm-modules/terraform-ibm-cloud-monitoring/blob/main/modules/metrics_routing/README.md
• I think you should rename the submodule: modules/scc_wp_config_aggregator -> modules/config_aggregator (drop the scc_wp_ prefix)
• I'm not sure I understand the point of the appconfig example you added. It seems to be the same as the complete example. Don't you wan't to add a new example that tests your new config aggregator submodule? Usually I would say just append to the complete example, but I think that will be a problem with our current tests that runs the example as its not run in an enterprise account. So I suggest you create a new example called config-aggregator that provisions an App Config instance, the required trusted profile resource and the config-aggregator.

[ocofaigh]

@ctolon22 @RiadhJouini Should the submodule be more generic? For example you are hard coding things like resource_collection_enabled = true and resource_collection_regions = ["all"]? Perhaps it should just be a generic config_aggregator submodule? Or perhaps it should be just a new feature to the root level module? Thoughts?

[ctolon22]

@ctolon22 @RiadhJouini Should the submodule be more generic? For example you are hard coding things like resource_collection_enabled = true and resource_collection_regions = ["all"]? Perhaps it should just be a generic config_aggregator submodule? Or perhaps it should be just a new feature to the root level module? Thoughts?

We could do it more general but maybe in a second phase.
The reason we need to have it as submodule is because it requires some trusted profiles so enabling this feature it needs to be done in 2 steps

[RiadhJouini]

Done these two: Can you please add a README.md file in the new submodule folder. As a reference, see: https://github.com/terraform-ibm-modules/terraform-ibm-cloud-monitoring/blob/main/modules/metrics_routing/README.md
I think you should rename the submodule: modules/scc_wp_config_aggregator -> modules/config_aggregator (drop the scc_wp_ prefix)

[RiadhJouini]

Let's keep in mind the dependencies between modules here. We need the app config generated before the trusted profiles as it will be referenced as a trusted relationship. Then we generate the templates and policies before the config aggregator, as the latter needs a the template ID for scoping/assigning templates the child accounts. So the overall order of the full chain deploy is the following: resource group - wp - app config - trusted profiles - trusted profiles template - config aggregator.

[RiadhJouini]

In answer to this comment: "I'm not sure I understand the point of the appconfig example you added. It seems to be the same as the complete example. Don't you wan't to add a new example that tests your new config aggregator submodule? Usually I would say just append to the complete example, but I think that will be a problem with our current tests that runs the example as its not run in an enterprise account. So I suggest you create a new example called config-aggregator that provisions an App Config instance, the required trusted profile resource and the config-aggregator." --> You're right, the appconfig folder served as a testing folder that was first called to avoid making any changes in the complete folder. As it's the same (and not called anymore), i deleted it. Config aggregator cannot be tested outside an enterprise account (at the moment)... cc @ctolon22

[ocofaigh]

As discussed today, there is no point in having a module which is a straight wrapper around a provider resource. We will just use provider directly in the workload protection end to end example.