nabsul / k8s-ecr-login-renew

Renews Docker login credentials for an AWS ECR container registry.
MIT License
205 stars 49 forks source link

🚸 Improve Usability for Multiline Target Namespace Declaration #24

Closed jeremyruffell closed 2 years ago

jeremyruffell commented 2 years ago

Summary

Added the ability to use Multiline YAML to declare Target Namespaces. When Declaring a large amount of namespaces having all the namespaces in one line can become hard to read, format and track on git.

I have added the formatNamespaceList function that replaces a new line with a comma & removes all whitespace. This now means we can use Multiline YAML whilst also retaining backwards compatibility and not introduce any breaking changes.

Examples

Testing

I have tested this using both the tests provided in this repo & using this application with these changes in a cluster with 30+ namespaces.

nabsul commented 2 years ago

I took a quick look at the change, and this looks super clear, straight forward, and fully backwards compatible. Thanks!

I might not get around to fully testing this till the weekend, but I don't expect to have any concerns around publishing this in an update.

(also, don't mind the failed build, that's probably my fault)

jeremyruffell commented 2 years ago

I took a quick look at change, and this looks super clear, straight forward, and fully backwards compatible. Thanks!

I might not get around to fully testing this till the weekend, but I don't expect to have any concerns around publishing this in an update.

(also, don't mind the failed build, that's probably my fault)

Cheers! Il send a PR your way fixing up the actions when I get a chance!

nabsul commented 2 years ago

Actually, should we add \t and r to the character list?

nabsul commented 2 years ago

Probably worth converting to a for loop at that point.

jeremyruffell commented 2 years ago

Actually, should we add \t and r to the character list?

Now you say it yep makes sense! want me to add these to the formatNamespaceList func:

And update this PR?

nabsul commented 2 years ago

I double checked the k8s docs to make sure that none of those characters are allowed in a namespace: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/

jeremyruffell commented 2 years ago

Updated PR to strip:

I will push up some changes to the Actions and Documentation on how to use with Multiline YAML soon 💜

nabsul commented 2 years ago

I messed up the history tracking, but everything in this PR is merged to #27 in preparation for releasing a new version of the tool.