Skip to content

Conversation

@vannicktrinquier
Copy link
Collaborator

No description provided.

@github-actions github-actions bot added the on:tools New or changed tool label Nov 30, 2025
Copy link
Collaborator

@ludoo ludoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is too large and tackles too many things, some of which need a design discussion.

Can you please split it so:

  • one PR adds support for KMS keys contexts to modules
  • one PR implements any change you need in the stage 0 dataset

Then we discuss why you need to add projects to the security stage, and why you're adding an additional project module call to the project factory. None of those look ok from my PoV.


# observability require IAM and service agent configured when using CMEK encryption
module "projects-observability" {
source = "../project"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should find a way of fitting this into the existing two project modules. Please let's discuss why you need to do this.

Copy link
Collaborator Author

@vannicktrinquier vannicktrinquier Nov 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We got a circular dependency between cmek permissions, log bucket creation using cmek and log based metrics using context, and context based on log bucket creation.

Something like:

  1. log based metrics require log buckets and use log bucket context
  2. log bucket context is build as follow (need module.log-buckets)
  ctx_log_buckets = merge(local.ctx.log_buckets, local.log_buckets)
  log_buckets = {
    for key, log_bucket in module.log-buckets : key => log_bucket.id
  }
  1. log bucket require cmek encryption key and iam stuff done for cmek so it depends_on module.projects-iam to get the expected permissions to role can be assigned to service agent. If log bucket is created before module.projects-iam, it will fails because cmek permission
  2. module.project-iam depends on log bucket context .... (as done initially)

So this was to break the circular dependency on log bucket which need permissions on cmek before being created. So log based metrics are created once bucket are created and once cmek is setup with iam

Let me try to see during the week and deep dive if i can avoid it. Open to ideas if any

@vannicktrinquier
Copy link
Collaborator Author

This PR is too large and tackles too many things, some of which need a design discussion.
Yes the plan was to create a draft to discuss the overall changes. Let me split between module on master vs hardened datasets stuff.

Then we discuss why you need to add projects to the security stage, and why you're adding an additional project module call to the project factory. None of those look ok from my PoV.

  • Regarding you need to add projects to the security stage,, if u mean about security projects added to var for project factory, i will remove it. No needed anymore (see above).
  • Regarding additional project module, i explained it above. Let see if we can find a better way

@vannicktrinquier vannicktrinquier changed the title Add controls related to CMEK encryption and support to FAST [WIP] Add controls related to CMEK encryption and support to FAST Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants