fix: update resource names to snake_case

[wyardley]

• Update resource names to snake_case
• Add moved blocks for resources and consolidate into moved.tf

I considered keeping - as a separator between the two parts for the google_project_iam_member resources, but ended up landing on just keeping everything snake case.

Followup to #2149

[apeabody]

/gcbrun

[wyardley]

@apeabody I checked, but may want to triple-check the moved {} blocks to make sure there are no typos.

I didn't do any functional testing, like bringing this up with the old version of the module, then updating to the new version.

[apeabody]

I wonder if this should be direct to kube_dns_upstream_nameservers? No sure if they support "chaining"?

Also very likely we won't have a release with the kube-dns-upstream-nameservers anyway.

[wyardley]

Fair. I'd assumed it would work since the "from" is different, so it would just relocate either that was found?

If you're sure they're going out together, I'll update it now to just have the one. 👍
cc: @MaximF

[apeabody]

What I don't know is if the move rules are "chained", if they are not and this rule fires, it will just try to move to kube-dns-upstream-nameservers, which of course no longer exists. As this PR also adds kube_dns_upstream_nameservers, regardless it should be perfectly safe to go directly to that resource.

[apeabody]

@apeabody I checked, but may want to triple-check the moved {} blocks to make sure there are no typos.

I didn't do any functional testing, like bringing this up with the old version of the module, then updating to the new version.

Yeah - Prior to merging we should do a quick migration test (create an example with tf apply using the previous version, then tf apply with this version)

[wyardley]

Are you setup to run that easily? I'm not sure if I'll be setup to test it quickly, though I can try if you need me to.

question: is there a way to turn on the tflint rule for resource naming specific to this module somehow, to prevent new resources getting created with upper case letters or hyphens? I think this is mostly using Google's internal and somewhat minimal tflint config, right?

[apeabody]

Are you setup to run that easily? I'm not sure if I'll be setup to test it quickly, though I can try if you need me to.

question: is there a way to turn on the tflint rule for resource naming specific to this module somehow, to prevent new resources getting created with upper case letters or hyphens? I think this is mostly using Google's internal and somewhat minimal tflint config, right?

:) I was also thinking if we can use tflint, as I believe this rule can be configured to work: https://github.com/terraform-linters/tflint-ruleset-terraform/blob/main/docs/rules/terraform_naming_convention.md

This default tflint config for modules can be found here: https://github.com/GoogleCloudPlatform/cloud-foundation-toolkit/blob/master/infra/build/developer-tools/build/home/.tflint.module.hcl

However I also recently added the option for local repo tflint configs, especially given the large number of repos that would likely also need to be updated first: https://github.com/GoogleCloudPlatform/terraform-google-enterprise-application/blob/main/.github/.tflint.repo.hcl

As far as testing, I was planning to run a few examples either tomorrow or Friday.

[apeabody]

/gcbrun

[apeabody]

I did a quick test with simple_autopilot_private and it looked good

  # module.gke.google_project_iam_member.cluster_service_account-metric_writer[0] has moved to module.gke.google_project_iam_member.cluster_service_account_metric_writer[0]
    resource "google_project_iam_member" "cluster_service_account_metric_writer" {
        id      = "-/roles/monitoring.metricWriter/serviceAccount:[email protected]"
        # (4 unchanged attributes hidden)
    }

  # module.gke.google_project_iam_member.cluster_service_account-nodeService_account[0] has moved to module.gke.google_project_iam_member.cluster_service_account_node_service_account[0]
    resource "google_project_iam_member" "cluster_service_account_node_service_account" {
        id      = "-/roles/container.defaultNodeServiceAccount/serviceAccount:[email protected]"
        # (4 unchanged attributes hidden)
    }

  # module.gke.google_project_iam_member.cluster_service_account-resourceMetadata-writer[0] has moved to module.gke.google_project_iam_member.cluster_service_account_resource_metadata_writer[0]
    resource "google_project_iam_member" "cluster_service_account_resource_metadata_writer" {
        id      = "-/roles/stackdriver.resourceMetadata.writer/serviceAccount:[email protected]"
        # (4 unchanged attributes hidden)
    }
    ```

[MaximF]

@wyardley Thanks for the pushing moved.tf changes so promptly! 👍
@apeabody I like the idea of using tflint for enforcing the snake_style naming usage!

[apeabody]

/gcbrun

[apeabody]

  # module.gke.kubernetes_config_map_v1_data.kube-dns-upstream-namservers[0] has moved to module.gke.kubernetes_config_map_v1_data.kube_dns_upstream_nameservers[0]
    resource "kubernetes_config_map_v1_data" "kube_dns_upstream_nameservers" {
        id            = "kube-system/kube-dns"
        # (3 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

[wyardley]

BTW, we can update the commit prefix to refactor if you'd rather it not show up in the changelog - it in theory shouldn't affect end-users, but I guess may be good to call it out in the changelog just in case?

[apeabody]

BTW, we can update the commit prefix to refactor if you'd rather it not show up in the changelog - it in theory shouldn't affect end-users, but I guess may be good to call it out in the changelog just in case?

When in doubt I prefer to include in the release notes. If end users do have resources that will be migrated they will see the "moved" in their first subsequent apply, although no changes to the actual resources.

[apeabody]

/gcbrun

[apeabody]

Thanks for the contribution @wyardley!