Skip to content
This repository was archived by the owner on Apr 15, 2024. It is now read-only.

Docker registry default #657

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
"ecmaVersion": 2022
},
"extends": "airbnb-base",
"parserOptions": {
Copy link
Contributor

Choose a reason for hiding this comment

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

unsure if this is still needed. maybe not? @jgielstra-cs

"ecmaVersion": 2020
},
"rules": {
"no-underscore-dangle": [
0
Expand Down
98 changes: 66 additions & 32 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 12 additions & 6 deletions src/commands/docker.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
import debugSetup from 'debug';
import URL from 'url-parse';
import { printSuccess, printError, callMe } from './utils.js';
import { generateJwt, loadProfile } from '../config.js';
/*
* Copyright 2023 Cognitive Scale, Inc. All Rights Reserved.
*
Expand All @@ -17,7 +13,12 @@ import { generateJwt, loadProfile } from '../config.js';
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import debugSetup from 'debug';
import URL from 'url-parse';
import { printSuccess, printError, callMe } from './utils.js';
import { generateJwt, loadProfile } from '../config.js';
const debug = debugSetup('cortex:cli');

export default class DockerLoginCommand {
constructor(program) {
this.program = program;
Expand All @@ -27,8 +28,13 @@ export default class DockerLoginCommand {
const profile = await loadProfile(options.profile);
const ttl = options.ttl || '14d';
try {
// TODO fetch this from new endpoint or maybe store this in the profile??
const registryUrl = (new URL(profile.url)).hostname.replace('api', 'private-registry');
// First try to see if we have of registries from /v4/info, grab the first one with isCortex == true
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want any unit tests to check these scenarios?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah need to add some... I just added a quick fix cause it was blocking me from some other testing..

let registryUrl = Object.values(profile?.registries ?? {}).find((v) => v?.isCortex === true)?.url;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If a cluster has 2 or more registries(one of them being private-registry.) then we are choosing the default only.. Maybe a warning to the user that a certain registry is being used of the 2 or more options.
The below warning comes up ONLY when profile.registries.url is undefined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know what a good alternative is though... unless we want to add a registry to cortex configure &&|| `cortex workspaces --registry so you can specify one. ??

Just using string replace isn't good enough though

// If not found try to guess the url, assuming our recommended install api.<my dns> and private-registry.<my dns>
if (registryUrl === undefined) {
registryUrl = (new URL(profile.url)).hostname.replace('api', 'private-registry');
printWarning(`Using default docker registry url: ${registryUrl}`);
}
const jwt = await generateJwt(profile, ttl);
const command = `docker login -u cli --password ${jwt} ${registryUrl}`;
debug('%s.executeDockerLogin(%s)', profile.name, command);
Expand Down