coder / modules

A collection of Terraform Modules to extend Coder templates.
https://registry.coder.com
Apache License 2.0
33 stars 33 forks source link

feat(jfrog): support multiple repositories #289

Closed BrentSouza closed 1 month ago

BrentSouza commented 2 months ago

This MR addresses two items:

  1. There is a bug in run.sh that keeps adding autocompletion to either .bashrc or .zshrc
  2. Allow users to specify multiple repositories for a given artifact type

Autocompletion

The run.sh script is looking for the wrong string to check if autocompletion has already been applied. The changes here correct that so autocompletion stanzas are only added once to the relevant shell startup script.

Multiple Repos

With Artifactory, you can have multiple repositories of a given type. For example, at my company, we promote docker images across environment specific repositories to keep our production image repository clean. It would be nice to setup auth for each of these instead of picking just one. The changes here allow for a list of repos to be defined. The first will function as a global repository, with varying behavior for the others based on the type.

pypi

The first repo is added to pip.conf as index-url, while any additional repos are added to extra-index-url

npm

The first repo is added as a default registry. Subsequent repos should be scoped, e.g. @foo:npm-repo-name and they will be added as such to .npmrc

docker

Attempts to authenticate to all defined repositories

go

Adds all repos to GOPROXY

I've added some tests to check the various output in the generated script, but I still need to validate that the module works as intended in our coder deployment. I wanted to open this pull request now to get any potential feedback on the changes. If this is ok, I'll also extend the changes to the jfrog-oauth module.

BrentSouza commented 2 months ago

Tested the module from my branch in our deployment and it's working at least how I intended! Please let me know if there are any changes you would like made.

matifali commented 2 months ago

Thank you, @BrentSouza, for the contribution. We have two modules for JFrog; one uses OAuth, and the other uses an admin token to create user-scoped access tokens. Would it be possible for you to port all these changes to the OAuth module, too? Again this is a great addition.

BrentSouza commented 2 months ago

Thanks @matifali! Yes I will happily make the changes to jfrog-oauth. Do you want a separate PR for that one?

matifali commented 2 months ago

Let's keep them in a single PR.

matifali commented 2 months ago

Once you are done with the changes. I can test both of them and merge the PR. Again thanks a lot for the contribution.

BrentSouza commented 2 months ago

@matifali at your convenience, please take a look at the port to jfrog oauth 😄. I can't test this one locally as we are on coder community edition and already have external auth configured with our GitLab instance.

BrentSouza commented 2 months ago

Actually I found a bug already 😱 Patch incoming

BrentSouza commented 2 months ago

Ok @matifali I think we should have successful test and pretty runs now

matifali commented 2 months ago

@BrentSouza Thank you for the contribution. This is a huge enhancement.

BrentSouza commented 2 months ago

I've addressed all concerns in the latest commit. I had to make some changes to test.ts to support typed vars since the method signature for testRequiredVariables was Record<string, string>

BrentSouza commented 1 month ago

No thanks necessary. Thanks for the taking the time to give valuable feedback!