deploy-cloudrun icon indicating copy to clipboard operation
deploy-cloudrun copied to clipboard

Sanitise service name and GCP-imposed rules

Open erzz opened this issue 3 years ago • 5 comments

TL;DR

I would like the possibility (probably an input like sanitize-values: true) to sanitize the inputs based on GCP naming restrictions.

A prime example is that we produce branch-based deployments and it is common that a branch might be named bug/TICKET-1234

So we produce the name of the service along the lines of ${{ github.ref_name }}-${{ github.event.repository.name }} but in the case above that would produce bug/TICKET-1234-myapplication where / is obviously not allowed.

The other example would be when using something like Renovate which can produce very long branch names meaning the final cloud run service name is >63 characters

Detailed design

The below input would apply a "slugging" of values such as bug/TICKET-1234-myapplication to bug-ticket-1234-myapplication

It would also trim any result to the max allowed characters for the value

inputs.sanitize-values: true (default false)

Additional information

No response

erzz avatar Jan 21 '22 08:01 erzz

Hi @erzz

I'm just one opinion, but I'm generally averse to client-side validation and manipulation of values. The server should really be enforcing this. If the allowed values were to change in the future, our Action would be unnecessarily limiting or modifying the resource name. We couldn't easily remove the validation, because that would be a breaking change for existing users.

Ideally GitHub Actions would have native functions for toLower and trim, but they do not.

I took a look at what gcloud does, and it looks like there is some client-side validation there:

$ gcloud run deploy "banana apple" --image gcr.io/vargolabs/gcr-cleaner --project vargolabs
ERROR: (gcloud.run.deploy) Invalid resource name [banana apple]. The name must use only lowercase alphanumeric characters and dashes, cannot begin or end with a dash, and cannot be longer than 63 characters.

Since deploy-cloudrun calls gcloud under the hood, you should be getting a similar error.

sethvargo avatar Jan 21 '22 14:01 sethvargo

Hi @sethvargo !

Ideally GitHub Actions would have native functions for toLower and trim, but they do not

Amen to that! Extremely frustrating!. Likewise, no slug-variables, no short_sha's, and more ....

But isn't that the utility of an action? That it can remove the laborious and decidedly "un-DRY" workflows you are left with when working with these limitations and need to insert custom scripts here and there (in all kinds of horrible implementations) to handle things like this?

I mean:

  • option 1: I pull in yet another action based on a 200mb docker image to create slugs :D
  • option 2: I start hacking away with shell expansions, regex, cut and trim to get the dynamic naming to conform to the google (well RFC-819 I guess) standard
  • option 3: the provider of the service knows the standard, has built the action, and can do the sanitisation inline if I opt-in with a boolean input?

We want to dynamically relate the name of the deployment to the branch for obvious reasons. But even when it comes to a release where we typically want to name a revision after the release tag v1.2.3 we again need to add in a sed to convert . to -

Just saying and of course :trollface: I respect the decision either way!

erzz avatar Jan 24 '22 22:01 erzz

But isn't that the utility of an action?

An action, but I don't think this action. I'm a big fan of the "do one thing and do it right" philosophy.

  • option 1: I pull in yet another action based on a 200mb docker image to create slugs :D

I think it's possible to do this in significantly less than 200mb. It would require another action, but that seems to be pretty common in the Actions ecosystem. I can envision a "slugify" action being generally useful.

sethvargo avatar Jan 24 '22 22:01 sethvargo

Untested, but something like this should work:

- id: 'slugify'
  run: |-
    node -c 'const input = "${{ my-input }}".replace(/W+/, '-').substring(0, 30); process.stdout.write("::set-output name=slug::"+x)'

sethvargo avatar Jan 24 '22 22:01 sethvargo

Yes of course there are smaller / slimmer actions too :) But engineers pick generally the first that they find :)

Nice node version by the way!

But yes on the topic of do one thing and do it well ... surely validation and sanitisation are components of doing any kind of code well? :)

But sure you can also argue that actions are like functions and perhaps you would pass off validation to another function.

Would be interested to hear others opinions too - there is no rush in this :)

erzz avatar Jan 25 '22 08:01 erzz

Hey @erzz - I feel like this needs to be a top-level function of GitHub Actions, since it's basic string manipulation and not specific to this action.

sethvargo avatar Nov 29 '22 18:11 sethvargo