Skip to content

chore: pin version in simple_regional_with_ipv6 example #2115

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

Merged

Conversation

wyardley
Copy link
Contributor

@wyardley wyardley commented Sep 25, 2024

This resolves a warning that comes up when running the lint checks locally, and seems to bring it in line with other examples

The other thing I noticed when running the lint script locally from a fork is that it actually locally updates the module source when it's defined like this (e.g., examples/simple_fleet_app_operator_permissions/main.tf, and one other):

module "permissions" {
  source = "../../modules/fleet-app-operator-permissions"
  # also no version specification here

it will get updated to this somewhere in the process -- not sure where, but maybe that's also something that should be fixed?

--- a/examples/autopilot_private_firewalls/main.tf
+++ b/examples/autopilot_private_firewalls/main.tf
@@ -33,7 +33,7 @@ provider "kubernetes" {
 }
 
 module "gke" {
-  source                            = "../../modules/beta-autopilot-private-cluster/"
+  source                            = "wyardley/kubernetes-engine/google//modules/beta-autopilot-private-cluster"
   project_id                        = var.project_id
   name                              = "${local.cluster_type}-cluster"
   regional                          = true

Does it make sense to reference this module the same way as the other examples have it (e.g., terraform-google-modules/kubernetes-engine/google//modules/fleet-app-operator-permissions), or will this cause unintended consequences in this case? I could add the two that are specified that way here as well if it makes sense.

This resolves a warning that comes up when running the lint checks
locally, and seems to bring it in line with other examples
@wyardley wyardley requested review from ericyz, gtsorbo and a team as code owners September 25, 2024 16:15
@apeabody
Copy link
Collaborator

/gcbrun

Copy link
Collaborator

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @wyardley!

Yeah, likely just an oversight.

@apeabody apeabody merged commit 7ea1752 into terraform-google-modules:master Sep 25, 2024
4 checks passed
@wyardley wyardley deleted the wyardley/pin_version branch September 25, 2024 19:18
@wyardley
Copy link
Contributor Author

@apeabody any thought on the other issue mentioned in the issue description? Should those two examples be updated to use the same source style as the rest, or is it different for those modules?

@apeabody
Copy link
Collaborator

apeabody commented Sep 25, 2024

@apeabody any thought on the other issue mentioned in the issue description? Should those two examples be updated to use the same source style as the rest, or is it different for those modules?

Apologies - Yes, by default everything in the /examples folder should use the registry and version constraint similar to:

source  = "terraform-google-modules/kubernetes-engine/google"
version = "~> 33.0"

During the test and lint these are automatically replaced with referential paths so the local/PR version is used. These edits should then be automatically reverted, but if force quit or an unexpected error, that may not occur.

@wyardley
Copy link
Contributor Author

I only saw the issues with the ones with source = "../..". I can make a separate PR to adjust those two sneaky ones if it's not too much noise

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 this pull request may close these issues.

2 participants