Filter registry credentials by language - #2680
marcogario merged 3 commits into
Conversation
Sorry, something went wrong.
| const KEY_EXPIRY_YEARS = 2; | ||
|
|
||
| const LANGUAGE_TO_REGISTRY_TYPE = { | ||
| "java-kotlin": "maven_repository", |
There was a problem hiding this comment.
Should you add kotlin as a standalone language?
Also there is languages.ts, which is where we've traditionally added language-specific logic. I think I'd prefer to move this declaration to that file.
Sorry, something went wrong.
There was a problem hiding this comment.
I've pushed a commit where this logic better integrates with languages.ts, by reusing the Language type defined there.
I am 50/50 on whether we should move this map there. The main reason against doing so is that this information is really only useful for the proxy action and even the names of the registry types are fairly arbitrary and dependent on the specifics of the proxy binary. WDYT?
Sorry, something went wrong.
There was a problem hiding this comment.
I'm fine if the logic doesn't go into language.ts.
Sorry, something went wrong.
aeisenberg
left a comment
There was a problem hiding this comment.
Looks good. I have a suggestion, but it is purely stylistic and non-blocking.
Sorry, something went wrong.
| actions: "", | ||
| cpp: "", | ||
| go: "", | ||
| swift: "", |
There was a problem hiding this comment.
nit:
| actions: "", | |
| cpp: "", | |
| go: "", | |
| swift: "", | |
| actions: undefined, | |
| cpp: undefined, | |
| go: undefined, | |
| swift: undefined, |
Sorry, something went wrong.
| const registryTypeForLanguage = language | ||
| ? LANGUAGE_TO_REGISTRY_TYPE[language] | ||
| : undefined; |
There was a problem hiding this comment.
nit: If you apply the suggestion above, then this becomes simpler:
| const registryTypeForLanguage = language | |
| ? LANGUAGE_TO_REGISTRY_TYPE[language] | |
| : undefined; | |
| const registryTypeForLanguage = LANGUAGE_TO_REGISTRY_TYPE[language]; |
Sorry, something went wrong.
There was a problem hiding this comment.
I still need the check on language itself, though.
Sorry, something went wrong.
Introduce the language parameter for the start-proxy action so we know which credentials we should use.
By specifying the language with disambiguating cases where the same registry has distinct credentials. For example, in Artifactory it is possible to use a token that is scoped to the nuget feed, while the maven repository uses the username/password combination.
Merge / deployment checklist