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

gcloud-sqlproxy - Duplicate Port Name if instanceShortName is not defined #143

Closed
danyworks opened this issue Oct 18, 2023 · 0 comments · Fixed by #144
Closed

gcloud-sqlproxy - Duplicate Port Name if instanceShortName is not defined #143

danyworks opened this issue Oct 18, 2023 · 0 comments · Fixed by #144

Comments

@danyworks
Copy link
Contributor

danyworks commented Oct 18, 2023

Bug in gcloud-sqlproxy chart

Used Tools:
Google Kubernetes Engine Version: v1.27.3-gke.1700
Helm Version: v3.11.3
Helm Terraform Provider Version: 2.11.0

Description:
This bug affects gcloud-sqlproxy chart, because of this I could neither install this chart as a new release nor upgrade an existing one.
The values.yaml of the release, looked like this

cloudsql:
  instances:
    - instance: "long-instance-name-foo"
      project: "project"
      region: "region"
      port: 5432
    - instance: "long-instance-name-bar"
      project: "project"
      region: "region"
      port: 5433
# other values....

The helm release install/upgrade had failed with the error message " Duplicate Values for port name in deployment.yaml "

snippet from error message:
* Service "sqlproxy" is invalid: [spec.ports[1].name: Duplicate value: "long-instance-n", spec.ports[2].name: Duplicate value: "long-instance-n"] * Deployment.apps "sqlproxy" is invalid: [spec.template.spec.containers[0].ports[1].name: Duplicate value: "long-instance-n", spec.template.spec.containers[0].ports[2].name: Duplicate value: "long-instance-n"]

My analysis on this issue:

if the instanceShortName is not explicitly set, the chart creates instanceShortName using the first 15 characters of instance
and the created instanceShortName is no more unique if two instances have identitical prefix names of length 15 characters or longer.
As in our case we had multiple cloudsql instances with the same prefix and the prefix was longer than 15 characters.

What you expected to happen:
When the sqlproxy is deployed, it should have generated a random alphanumeric string of 9 or less digits and have appended it to instance name, so to create a unique instanceShortName

instanceShortName in helpers.tpl (gcloud-sqlproxy Chart Version 0.25.2):

{{/*
Create the short instance name
*/}}
{{- define "gcloud-sqlproxy.instanceShortName" -}}
{{ .instanceShortName |  default ((.instance |  trunc 15 | trimSuffix  "-" ) }}
{{- end -}} 

Suggested Fix (gcloud-sqlproxy Chart Version 0.25.3):

{{- define "gcloud-sqlproxy.instanceShortName" -}}
{{- $randomString := randAlphaNum 9 | lower -}}
{{ .instanceShortName | default (printf "%s-%s" (.instance | trunc 5 | trimSuffix "-") $randomString) }}
{{- end -}}

as a result, instanceShortName looks like this
image

How to reproduce it:


Deploy the chart using helm (even without terraform helm provider), with the following values but the only condition is to have the prefix on more than one instances, and also the prefix should be more than 15 characters long

  cloudsql:
      instances:
        - instance: "long-instance-name-foo" # or any other name you prefer 
          project: "project"
          region: "region"
          port: 5432
        - instance: "long-instance-name-bar" # or any other name you prefer 
          project: "project"
          region: "region"
          port: 5433
danyworks pushed a commit to danyworks/charts that referenced this issue Oct 18, 2023
rimusz pushed a commit that referenced this issue Oct 19, 2023
…hortName (#144)

* fix #143 add randAlphaNum to instanceShortName

* gcloud-sqlproxy bump chart version to 0.25.3 and update readme

* gcloud-sqlproxy fix#143 allow only lowercase in randomString for instanceShortName

* gcloud-sqlproxy fix#143 reduce length

* gcloud-sqlproxy fix#143 update readme

---------

Co-authored-by: daniyal ibrahim <daniyal.ibrahim@capgemini.com>
rimusz pushed a commit that referenced this issue Oct 20, 2023
* fix #143 add randAlphaNum to instanceShortName

* gcloud-sqlproxy bump chart version to 0.25.3 and update readme

* gcloud-sqlproxy fix#143 allow only lowercase in randomString for instanceShortName

* gcloud-sqlproxy fix#143 reduce length

* gcloud-sqlproxy fix#143 update readme

* gcloud-sqlproxy fix #145 generate randomstring using sha1sum not with randAlphaNum

* gcloud-sqlproxy fix #145 update readme

* gcloud-sqlproxy fix #145 bump chart version

* add new line at the end of Chart.yaml

---------

Co-authored-by: daniyal ibrahim <daniyal.ibrahim@capgemini.com>
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.

1 participant