Skip to content

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 a team, ericyz and gtsorbo 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