Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stages: document how the new factory_data is meant to be used #1628

Closed
gustavovalverde opened this issue Aug 28, 2023 · 14 comments · Fixed by #1659
Closed

stages: document how the new factory_data is meant to be used #1628

gustavovalverde opened this issue Aug 28, 2023 · 14 comments · Fixed by #1659
Assignees

Comments

@gustavovalverde
Copy link
Contributor

Previously I just had to terraform apply with a defaults.yaml inside ./data/ and each project yaml inside ./data/projects, which has been working as expected. Until this PR was merged:

Now I'm specifying the path of the projects yaml as:

factory_data = {
  data_path = "./data/projects"
}

But after doing a terraform apply the plan shows it will remove all the previous configurations.

@ludoo
Copy link
Collaborator

ludoo commented Aug 28, 2023

Yes, the new version uses a different format for both projects and defaults. It's not backward compatible.

@gustavovalverde
Copy link
Contributor Author

Is this documented somewhere?

@ludoo
Copy link
Collaborator

ludoo commented Aug 28, 2023

The usual variables table at the bottom gives you the defaults, the sample in the README shows a simplified YAML format. The new project factory only proxies via YAML to the project module, so you can use that as a reference.

@gustavovalverde
Copy link
Contributor Author

After this last reply #1628 (comment) I've just realized you're refering to the README in https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/tree/master/blueprints/factories/project-factory

I was expecting those instructions in the stage https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/tree/master/fast/stages/3-project-factory/dev as the stages commonly have their instructions isolated.

@ludoo
Copy link
Collaborator

ludoo commented Aug 28, 2023

Ahhh good point, let's reopen this to keep track of improvements to the stage README. And thanks for raising this!

@ludoo ludoo reopened this Aug 28, 2023
@gustavovalverde
Copy link
Contributor Author

A few things to consider when updating the documentation and files in ../stages/3-project-factory as some features are not longer supported in https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/blob/master/fast/stages/3-project-factory/dev/main.tf like:

  • Billing alerts
  • Org Policies

This files seems like it needs to be changed, as this format is not being mapped correctly: https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/blob/master/fast/stages/3-project-factory/dev/data/projects/project.yaml.sample

I'm even getting module.projects["<project>"].module.project.google_project.project[0] will be destroyed

@gustavovalverde
Copy link
Contributor Author

This might be relevant or not:

│ Error: Error in function call
│ 
│   on ../../../../blueprints/factories/project-factory/factory.tf line 36, in locals:
│   36:       contacts = coalesce(
│   37:         var.data_overrides.contacts,
│   38:         try(v.contacts, null),
│   39:         var.data_defaults.contacts
│   40:       )
│     ├────────────────
│     │ v.contacts is tuple with 1 element
│     │ var.data_defaults.contacts is empty map of list of string
│     │ var.data_overrides.contacts is null
│ 
│ Call to function "coalesce" failed: all arguments must have the same type.
╵
╷
│ Error: Error in function call
│ 
│   on ../../../../blueprints/factories/project-factory/factory.tf line 84, in locals:
│   84:       service_accounts = coalesce(
│   85:         var.data_overrides.service_accounts,
│   86:         try(v.service_accounts, null),
│   87:         var.data_defaults.service_accounts
│   88:       )
│     ├────────────────
│     │ v.service_accounts is object with 2 attributes
│     │ var.data_defaults.service_accounts is empty map of object
│     │ var.data_overrides.service_accounts is null

@juliocc
Copy link
Collaborator

juliocc commented Aug 28, 2023

@gustavovalverde this is @ludoo's thing and he's OOO. How urgent is this for you?

@gustavovalverde
Copy link
Contributor Author

It's urgent for me (as it's halting a few deployments I have pending), but it's not affecting any production environment right now as the changes can't be applied anyways.

If this could take a few days I'll just revert to a previous commit and re-apply some changes, until this can be looked at.

@ludoo ludoo self-assigned this Sep 7, 2023
@ludoo
Copy link
Collaborator

ludoo commented Sep 7, 2023

@gustavovalverde can you share (obfuscated as needed) configuration to replicate the errors you are showing above?

@gustavovalverde
Copy link
Contributor Author

gustavovalverde commented Sep 7, 2023

I'll share what I think are the relevant parts (not the whole config on some cases), if you need something else, just let me know:

0-boostrap:

fast_features = {
  data_platform   = false
  gke             = true
  project_factory = true
  sandbox         = false
  teams           = true
}

1-resman:

team_folders = {
  engineering = {
    descriptive_name = "XXX"
    group_iam = null
    impersonation_groups = ["XXX"]
  }
  operations = {
    descriptive_name = "XXX"
    group_iam = null
    impersonation_groups = ["XXX"]
  }
}

tenants = {
  services = {
    admin_group_email = "XXX"
    descriptive_name  = "Services Tenant"
  }
}
tenants_config = {
  core_folder_roles = [
    "roles/compute.instanceAdmin.v1",
  ]
  top_folder_roles = ["roles/logging.admin", "roles/monitoring.admin"]
}

2-networking-d-separate-envs:

vpn_onprem_dev_primary_config = null
vpn_onprem_prod_primary_config = null

vpn_onprem_configs = null

regions = {
  primary = "XXX"
}

3-project-factory:

factory_data = {
  data_path = "data/projects"
}

data/projects/<project>.yaml

# skip boilerplate check

# [opt] Billing alerts config - overrides default if set
billing_alert:
  amount: 1000
  thresholds:
    current:
      - 0.8
      - 1.0
    forecasted: []
  credit_treatment: INCLUDE_ALL_CREDITS

# [opt] DNS zones to be created as children of the environment_dns_zone defined in defaults
dns_zones: []

# [opt] Contacts for billing alerts and important notifications
essential_contacts:
  - XXX

# Folder the project will be created as children of
folder_id: folders/XXX

# [opt] Authoritative IAM bindings in group => [roles] format
group_iam:
  XXX:
    - roles/editor

# [opt] Authoritative IAM bindings in role => [principals] format
# Generally used to grant roles to service accounts external to the project
iam:
  roles/iam.workloadIdentityUser:
    - principalSet://iam.googleapis.com/projects/XXX/locations/global/workloadIdentityPools/XXX-bootstrap/*
  roles/editor:
    - serviceAccount:[email protected]

# [opt] Labels for the project - merged with the ones defined in defaults
labels:
  environment: dev
  application: zebra

# [opt] Org policy overrides defined at project level
org_policies:
  compute.disableGuestAttributesAccess:
    rules:
    - enforce: false
  compute.trustedImageProjects:
    rules:
    - allow:
        values:
        - projects/cos-cloud
        - projects/dataflow-service-producer-prod
        - projects/serverless-vpc-access-images
        - projects/windows-cloud
  compute.vmExternalIpAccess:
    rules:
      - allow:
          all: true
  iam.allowServiceAccountCredentialLifetimeExtension:
    rules:
      - allow:
          all: true

# [opt] Service account to create for the project and their roles on the project
# in name => [roles] format
service_accounts:
  XXX:
    - roles/compute.instanceAdmin
    - roles/compute.storageAdmin
    - roles/compute.loadBalancerAdmin
    - roles/errorreporting.user
    - roles/logging.logWriter
    - roles/monitoring.metricWriter
    - roles/artifactregistry.reader
    - roles/iam.serviceAccountUser
    - roles/iam.workloadIdentityUser
  XXX:
    - roles/artifactregistry.writer
    - roles/iam.workloadIdentityUser

# [opt] APIs to enable on the project.
services:
  - artifactregistry.googleapis.com
  - compute.googleapis.com
  - clouderrorreporting.googleapis.com
  - cloudresourcemanager.googleapis.com
  - containeranalysis.googleapis.com
  - logging.googleapis.com
  - monitoring.googleapis.com
  - osconfig.googleapis.com
  - networkmanagement.googleapis.com
  - stackdriver.googleapis.com
  - storage.googleapis.com
  - iap.googleapis.com

# [opt] Roles to assign to the service identities in service => [roles] format
service_identities_iam:
  compute:
    - roles/storage.objectViewer

  # [opt] VPC setup.
  # If set enables the `compute.googleapis.com` service and configures
  # service project attachment

vpc:
  # [opt] If set, enables the container API
  gke_setup: null

  # Host project the project will be service project of
  host_project: XXX-dev-net-spoke-0

  # [opt] Subnets in the host project where principals will be granted networkUser
  # in region/subnet-name => [principals]
  subnets_iam:
    us-east1/dev-default-ue1:
      - user:XXX
      - serviceAccount:[email protected]

@ludoo
Copy link
Collaborator

ludoo commented Sep 8, 2023

│ Error: Error in function call
│ 
│   on ../../../../blueprints/factories/project-factory/factory.tf line 36, in locals:
│   36:       contacts = coalesce(
│   37:         var.data_overrides.contacts,
│   38:         try(v.contacts, null),
│   39:         var.data_defaults.contacts
│   40:       )
│     ├────────────────
│     │ v.contacts is tuple with 1 element
│     │ var.data_defaults.contacts is empty map of list of string
│     │ var.data_overrides.contacts is null
│ 
│ Call to function "coalesce" failed: all arguments must have the same type.

Your YAML file is passing a list of contacts where a map is needed. You can refer to the project module documentation for the exact type.

│ Error: Error in function call
│ 
│   on ../../../../blueprints/factories/project-factory/factory.tf line 84, in locals:
│   84:       service_accounts = coalesce(
│   85:         var.data_overrides.service_accounts,
│   86:         try(v.service_accounts, null),
│   87:         var.data_defaults.service_accounts
│   88:       )
│     ├────────────────
│     │ v.service_accounts is object with 2 attributes
│     │ var.data_defaults.service_accounts is empty map of object
│     │ var.data_overrides.service_accounts is null

Same as above, your YAML is passing an invalid type.

@gustavovalverde
Copy link
Contributor Author

I'm happy to see that Org Policies were reintroduced here: aa5d883

Will Billing Alerts also be reintroduced (as this was my main concern here) or should we implement this other way?

@ludoo
Copy link
Collaborator

ludoo commented Oct 19, 2023

I'm happy to see that Org Policies were reintroduced here: aa5d883

Will Billing Alerts also be reintroduced (as this was my main concern here) or should we implement this other way?

Hey Gustavo, we created a new billing account module and moved billing alerts functionality inside it, also thanks to your feedback.

We are now ready to look at how to reimplement that functionality. My preference would be to add billing alert factory support to the billing account module, so alerts can be specified via a dedicated set of yaml files, and then either referencing existing alerts from the project factory, or combining it so the project data source can feed and populate the billling alerts data source.

Let's discuss it in a new issue, I'll open it in a moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants