Skip to content

Conversation

nissessenap
Copy link
Contributor

This, to be able to have longer and unique names.
The firewall API supports 63 charters
Solves #1527

https://cloud.google.com/compute/docs/reference/rest/v1/firewalls

# The longest name with the extra chars added by the module echo gke--intra-cluster-egress |wc -c 26 # the max ammount of char - the number of chars the module creates expr 63 - 26 37 # I picked 36 since it looks better then 37 :)
@nissessenap nissessenap requested review from a team, Jberlinsky and ericyz as code owners May 25, 2023 08:13
@nissessenap nissessenap changed the title Set max firewall name to 36 fix: set max firewall name to 36 May 25, 2023
@bharathkkb
Copy link
Member

/gcbrun

Copy link
Member

@bharathkkb bharathkkb 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 PR @nissessenap

Looks like some of our tests may need updating

gcloud.Runf(t, "compute firewall-rules --project %s describe gke-%s-intra-cluster-egress", projectId, clusterName[:25])
gcloud.Runf(t, "compute firewall-rules --project %s describe gke-%s-webhooks", projectId, clusterName[:25])

This to be able to have longer and unique names. The firewall API supports 64 charters Solves terraform-google-modules#1527 Signed-off-by: Edvin Norling <edvin.norling@kognic.com>
Signed-off-by: Edvin Norling <edvin.norling@kognic.com>
@nissessenap
Copy link
Contributor Author

@bharathkkb I updated the tests as you pointed out

@bharathkkb
Copy link
Member

/gcbrun

@bharathkkb
Copy link
Member

bharathkkb commented Jun 14, 2023

Looks like 36 is OOB since the test cluster name is smaller but I think we can just skip truncating since it's a static size from example.

 --- FAIL: TestSaferCluster (30.46s) panic: runtime error: slice bounds out of range [:36] with length 26 [recovered]	panic: runtime error: slice bounds out of range [:36] with length 26 
@bharathkkb
Copy link
Member

/gcbrun

@bharathkkb bharathkkb merged commit 29d9259 into terraform-google-modules:master Jun 15, 2023
@nissessenap nissessenap deleted the 1527_traunc_fw branch June 15, 2023 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants