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

feat(helm): Add repo alias support #4844

Merged
merged 13 commits into from Nov 24, 2019
Merged

Conversation

timja
Copy link
Contributor

@timja timja commented Nov 21, 2019

Add repo alias support

Closes #4681

Note: Only tested via unit tests, any docs need updating?

@@ -18,6 +18,7 @@ export interface ExtractConfig extends ManagerConfig {
endpoint?: string;
global?: any;
gradle?: { timeout?: number };
helm?: { aliases?: Record<string, string> };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the right way to add additional config?
also there's this

"helm-requirements": {

but the key is helm-requirements which doesn't play well in javascript land, will it get auto converted to helmRequirements or am I on the wrong track?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can surround it in 'helm-requirements' anywhere necessary

@rarkins
Copy link
Collaborator

rarkins commented Nov 21, 2019

Thanks @timja. Can you give a description of how this is intended to work. e.g. are aliases needing to be configured manually in config, or can they be autodetected from the repo?

@timja
Copy link
Contributor Author

timja commented Nov 21, 2019

Thanks @timja. Can you give a description of how this is intended to work. e.g. are aliases needing to be configured manually in config, or can they be autodetected from the repo?

There's no way to detect them, we could add the stable one by default,
stable: https://kubernetes-charts.storage.googleapis.com/

It would be expected to add something like:

{
  "helm": {
    "aliases": {
      "my-company": "https://my-registry.gcr.io/",
      "stable": "https://kubernetes-charts.storage.googleapis.com/"
    }
  }
}

@timja
Copy link
Contributor Author

timja commented Nov 21, 2019

Also see this comment for more details:
#4681 (comment)

@rarkins
Copy link
Collaborator

rarkins commented Nov 21, 2019

Isn't it possible to detect aliases from elsewhere in the repo? We can manage it even if it's in different files.

i.e. isn't "my-company" in your example configured somewhere else within your repo, instead of each developer needing to manually provision it locally on their computer?

BTW we can't support unstructured key:value JSON configuration like you've done because it breaks JSON-schema validation.

@timja
Copy link
Contributor Author

timja commented Nov 21, 2019

Isn't it possible to detect aliases from elsewhere in the repo? We can manage it even if it's in different files.

#4681 (comment)
There's no standard unfortunately, each developer just does it once when they join and then it's there forever

BTW we can't support unstructured key:value JSON configuration like you've done because it breaks JSON-schema validation.

could you link to a sample please?

@rarkins
Copy link
Collaborator

rarkins commented Nov 22, 2019

I like the top level option name you suggested - aliases - because that name could be reused for other purposes later if other package managers have similar requirements. The problem with making aliases an object with key/value pairs as you suggest is that we have no way to formally validate them with JSON schema. If we instead made it an array of objects, it could be like this:

{
  "aliases": [
    {
      "aliasKey": "my-company",
      "aliasValue": "https://my-registry.gcr.io/"
    },
    {
      "aliasKey": "stable",
      "aliasValue": "https://kubernetes-charts.storage.googleapis.com/"
    }
  ]
}

This way our JSON schema can specify aliases, aliasKey and aliasValue and also that the latter two are to be strings. It's a bit less elegant though.

A second challenge is we'd want to make aliases overridable, e.g. a default "stable" value built into Renovate but allowing people to override it in their config. With a native object like you suggest then overriding would happen automatically when you merge the objects whereas it would need to be done explicitly with the above JSON schema approach.

@viceice @JamieMagee @felixfbecker do any of you have opinions on this?

@felixfbecker
Copy link

Pretty sure you can declare a JSON schema for an object map, e.g.

"properties" {
  "aliases": {
    "type": "object",
    "additionalProperties": {
      "type": "string",
      "format": "uri"
    }
  }
}

You can add a regex constraint to the key and value too.

@rarkins
Copy link
Collaborator

rarkins commented Nov 22, 2019

@felixfbecker that's great advice, thanks. I found a reference for it here: https://json-schema.org/understanding-json-schema/reference/object.html

Based on this I think we may be able to keep @timja's proposed config structure.

@timja
Copy link
Contributor Author

timja commented Nov 22, 2019

PTAL not sure if I've handled the schema side properly, but I tested the json schema with vs-code and it enforced the keyValue being a URL

@viceice
Copy link
Member

viceice commented Nov 22, 2019

@timja you have to update the schema again, pease run yarn create-json-schema and commit changes.

@rarkins I'm fine with the current alias definition.

lib/config/definitions.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move aliases default to inside helm-requirements

@timja timja requested a review from rarkins November 24, 2019 10:51
@rarkins rarkins merged commit cc07563 into renovatebot:master Nov 24, 2019
@rarkins
Copy link
Collaborator

rarkins commented Nov 24, 2019

Thanks @timja !

@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 19.69.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@timja timja deleted the helm-alias branch November 24, 2019 11:57
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support helm chart placeholder repositories
6 participants