GoogleCloudPlatform / cloud-foundation-fabric

End-to-end modular samples and landing zones toolkit for Terraform on GCP.
Apache License 2.0
1.51k stars 862 forks source link

FAST - project factory: inconsistency between one repo per environment vs subdirectories? #723

Closed mgfeller closed 2 years ago

mgfeller commented 2 years ago

The project factory code and its documentation suggested that one repository should be used for all environments, with projects for different environments specified in corresponding sub-directories.

However, the github workflow and cicd configuration suggest that different environments should be handled in separate repositories (variable cicd_repositories in 01-resman). The same applies to data platform.

Unless I have overlooked something, this does not appear to be consistent.

My impression is that a repo per environment would fit better in the FAST design as I understand it.

Either way, some changes appear to be needed. Also, a section in the documentation explaining this particular design choice would be helpful.

sruffilli commented 2 years ago

Ciao Micheal - as you say project factory is structured so that you have one per environment, to contain the IAM scope of the service account used to deploy projects, so if the documentation suggests otherwise, it needs to be fixed.

That said, we're planning to spend some time on the project factory to double check if its current implementation can be further simplified and streamlined, and the documentation better refined - and I'm looking forward for any comment you might have.

mgfeller commented 2 years ago

Ciao Simone - I'd be happy to contribute.

To clarify a bit, in my mind the variable declaration cicd_repositories in 01-resman does not correspond to the approach using subdirectories:

variable "cicd_repositories" {
  description = "CI/CD repository configuration. Identity providers reference keys in the `automation.federated_identity_providers` variable. Set to null to disable, or set individual repositories to null if not needed."
  type = object({
    ...
    project_factory_dev = object({
      branch            = string
      identity_provider = string
      name              = string
      type              = string
    })
    project_factory_prod = object({
      branch            = string
      identity_provider = string
      name              = string
      type              = string
    })
  ...
  })

Also, subdirectories are not reflected in the generated workflow files.

Maybe a new key path could be added to cicd_repositories entries and then used when generating the workflow files?

ludoo commented 2 years ago

To clarify a bit, in my mind the variable declaration cicd_repositories in 01-resman does not correspond to the approach using subdirectories:

Can you expand on why?

Maybe a new key path could be added to cicd_repositories entries and then used when generating the workflow files?

ci/cd variables are used to set Workload Identity Federation and IAM mappings, a path would not make much sense there. As in all the rest of FAST, we expect that some measure of customization is inevitable, do you think that workflow files should be treated differently?

Not stonewalling you of course, just trying to figure out what the use case is :)

mgfeller commented 2 years ago

Can you expand on why?

Because the 2 entries suggest - to me at least - 2 repositories, one for dev (project_factory_dev) and one for prod (project_factory_prod), or possibly 2 different branches.

ci/cd variables are used to set Workload Identity Federation and IAM mappings, a path would not make much sense there.

Yes, I can see that, at the same time cicd_repositories is also used when creating the workflow files, I guess that's why I suggested it.

As in all the rest of FAST, we expect that some measure of customization is inevitable, do you think that workflow files should be treated differently?

No, I don't. Based on my experience with other stages I run where everything works nicely out of the box (with necessary parameterization), I expected the same for ci/cd for project factory. When it didn't, I just started looking around...

Not stonewalling you of course, just trying to figure out what the use case is :)

No worries, I'm just trying to understand design and intention, so that I learn and customize in the right places :)

I'd be happy to contribute with a paragraph in the documentation about customizing the generated workflow files if that could be of help.

ludoo commented 2 years ago

Ok, I see your point. Sorry for being a bit disrupted but I've been traveling the past few days and had hectic hours. :)

Let me start by saying we did not spend a lot of cycles yet on how to design repositories and CI/CD for stage 3s, which are separate per environment.

My personal inclination (but again, it needs discussion and I might then get to see it differently) is to have a single repository for a set of stage 3 environments, e.g. project factory, which would host both the underlying module (pf, data platform, etc.) and one "wrapper module" per environment. Which as you observe matches what we wrote in the docs, but not what we have in code where we provide for one repo per environment.

So +1 from me on changing the shape of the CI/CD variables to account for that. But here things get a bit fuzzy (at least for me): how many and which branches, master for the underlying module then one branch per environment? and should we then have one workflow per environment branch, and one for master that only runs checks/tests? If that makes sense, it would mean changing the code to account for multiple workflows in stage 3s, which could get pretty hairy.

Let me know what you think, both on the general approach, the repo design for stage 3s, and of course if we should then complicate the code to have an out of the box solution (probably yes).

And thanks a lot for raising this issue.

mgfeller commented 2 years ago

Yes, it's a tricky task to solve, it can get hairy either way. It should be easy to reason about and understand. At the same it should also be practical to compare environments, and propagate changes e.g. from dev to prod and deploy them.

I think an out of the box solution would be really appreciated.

I'll try to structure this a bit in my mind and on paper and also ask colleagues. Given it is summer time this might take a while though - don't wait for me :)

ludoo commented 2 years ago

Well, we're in no rush there's a ton of things to do while we wait. :) Once you have made up your mind let's flesh out all the choices and decide how to design this properly, I'll be happy to have a discussion and will also involve a couple more people from our side who have some experience on this topic.

ludoo commented 2 years ago

@mgfeller actually the current design works for both scenarios, and I forgot it was done on purpose :)

Assume you want the same repo with one branch per environment, what you want is having the same federated identity configuration, but different conditions that allow impersonating the right SA based on branch. Which is what the current code allows you to do. The two separate workflow files are also generated for both branches. What is currently suboptimal is that both workflows assume a PR on dev/prod will merge to master, which might or might not be the case.

If you agree with what I outlined above, can we figure which parts of the documentation are misleading or incomplete, and what shape the workload files should have?

ludoo commented 2 years ago

Michael, I'm closing this as we're going through all open issues and trying to address them, and this has no pending actions. Feel free of course to reopen whenever you want to resume the discussion, and thanks as always for the insightful comments!

mgfeller commented 2 years ago

@ludoo, thanks for the clarification. just got back from vacation :)

yes, it makes sense if branches are used to distinguish between environments, in addition to the folders. not quite sure I considered that in detail, it's a while ago.

I'll be working with that over the next couple of weeks, having a closer look at a workflow using branches for the project factory. I think it would be useful to extend the documentation regarding this topic anyway. I'll see how it goes and then probably write something :)

ludoo commented 2 years ago

@mgfeller sounds great, and we can also have a chat in a meet all together if this makes it easier. Hope your vacation was great, I'm leaving myself eow but will still check things here, if you need to discuss!

mgfeller commented 2 years ago

Ludovico, thanks, I'll get back to this thread. Meanwhile, enjoy your vacation!