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

Allow use of private docker registries #1293

Merged
merged 14 commits into from Nov 26, 2019

Conversation

@shreddedbacon
Copy link
Member

shreddedbacon commented Oct 14, 2019

Checklist

  • Affected Issues have been mentioned in the Closing issues section
  • Documentation has been written/updated.
  • Changelog entry has been written

This change allows for private docker registries to be used to pull pre-built images into lagoon.

Changelog Entry

Feature - Allow the use of private docker registries to pull own images
Documentation - Configuration of private docker registries

@tobybellwood tobybellwood added this to Lagoon 1.2.0 (Nov 2019) in Lagoon Roadmap (Indicative) Oct 15, 2019
Copy link
Member

Schnitzel left a comment

just documentation review for now, didn't had time to check the code too much yet

```

And the service updated to use build instead of image

This comment has been minimized.

Copy link
@Schnitzel

Schnitzel Oct 23, 2019

Member

let's document here that we're talking about the docker-compose.yml file here


The `private-registries` block allows you to define your own private docker registries to pull private images. If you don't specify a `url` in your yaml, it will default to using docker hub.

The password can be defined as an environment variable name that is set in the Lagoon api [Environment Variables (Lagoon API) » Docker Registry Environment Variables (Lagoon API)](./environment_variables.md). If one is not set, it will default to the value defined in the `.lagoon.yml` file.

This comment has been minimized.

Copy link
@Schnitzel

Schnitzel Oct 23, 2019

Member

can you try explain that a bit better? honestly I'm confused where I should define the password and where.

This comment has been minimized.

Copy link
@shreddedbacon

shreddedbacon Oct 23, 2019

Author Member

Now that I read it back, I can see how its confusing. I'll word it better.

…could be some confusion around how to configure the password and how to actually consume a private image
@Schnitzel Schnitzel self-assigned this Nov 18, 2019
@Schnitzel Schnitzel added this to the v1.2.0 milestone Nov 18, 2019
Copy link
Member

Schnitzel left a comment

nice work! some small things

This defines a project wide docker registry variable (available in all environments) for the project with id `463`:
```
mutation addDockerRegistryEnv {
addEnvVariable(input:{type:PROJECT, typeId:463, scope:DOCKER, name:"MY_OWN_REGISTRY_PASSWORD", value:"MySecretPassword"}) {

This comment has been minimized.

Copy link
@Schnitzel

Schnitzel Nov 20, 2019

Member

I'm not sure about the name DOCKER here, I would have called it REGISTRY? also I think we should slowly get rid of Docker all together, as it might not exist for much longer, so maybe even CONTAINER_REGISTRY?

This comment has been minimized.

Copy link
@shreddedbacon

shreddedbacon Nov 20, 2019

Author Member

Yep, good idea. I think go with CONTAINER_REGISTRY. I'll make changes

fi
fi
done
set +x

This comment has been minimized.

Copy link
@Schnitzel

Schnitzel Nov 20, 2019

Member

this should probably be an set -x

@@ -541,7 +541,7 @@ CREATE OR REPLACE PROCEDURE
AND column_name = 'scope'
) THEN
ALTER TABLE `env_vars`
ADD `scope` ENUM('global', 'build', 'runtime') NOT NULL DEFAULT 'global';
ADD `scope` ENUM('global', 'build', 'runtime', 'docker') NOT NULL DEFAULT 'global';

This comment has been minimized.

Copy link
@Schnitzel

Schnitzel Nov 20, 2019

Member

this unfortunately is not gonna work. As the if in this procedure just checks if the scope column exist or not, and currently all lagoons will have that column already existing it will never run the ADD.

Instead we should create a new procedure which specifically checks if docker (or registry - see above) exists in the list of enums of column scope and if not add it.

This comment has been minimized.

Copy link
@Schnitzel

Schnitzel Nov 26, 2019

Member
Suggested change
ADD `scope` ENUM('global', 'build', 'runtime', 'docker') NOT NULL DEFAULT 'global';
ADD `scope` ENUM('global', 'build', 'runtime', 'container_registry') NOT NULL DEFAULT 'global';
@shreddedbacon

This comment has been minimized.

Copy link
Member Author

shreddedbacon commented Nov 20, 2019

I've renamed references to docker registries to container registries.

I wasn't sure on the procedure for modifying the enum so put an alter table in.

@@ -541,7 +541,7 @@ CREATE OR REPLACE PROCEDURE
AND column_name = 'scope'
) THEN
ALTER TABLE `env_vars`
ADD `scope` ENUM('global', 'build', 'runtime') NOT NULL DEFAULT 'global';
ADD `scope` ENUM('global', 'build', 'runtime', 'docker') NOT NULL DEFAULT 'global';

This comment has been minimized.

Copy link
@Schnitzel

Schnitzel Nov 26, 2019

Member
Suggested change
ADD `scope` ENUM('global', 'build', 'runtime', 'docker') NOT NULL DEFAULT 'global';
ADD `scope` ENUM('global', 'build', 'runtime', 'container_registry') NOT NULL DEFAULT 'global';
@@ -260,3 +260,42 @@ Each definition is keyed by a unique name (`secrets` and `logs-db-secrets` in th
* `path` - the path to the yaml file
* `command` - can either be `create` or `apply`, depending on if you like to run `kubectl create -f [yamlfile]` or `kubectl apply -f [yamlfile]`
* `ignore_error` - either `true` or `false` (default), this allows you to instruct the lagoon build script to ignore any errors that might are returned during running the command. (This can be useful to handle the case where you want to run `create` during every build, so that eventual configurations are created, but don't want to fail if they already exist).

#### `private-registries`

This comment has been minimized.

Copy link
@Schnitzel

Schnitzel Nov 26, 2019

Member

can we also call this container-registries? then it will be clean through the whole system, also this technically can be used for non-dockerhub but public registries, so using container-registries would fit even more :)

@shreddedbacon

This comment has been minimized.

Copy link
Member Author

shreddedbacon commented Nov 26, 2019

Have update the PR with the latest suggestions

@Schnitzel Schnitzel merged commit 0ef9b2b into amazeeio:master Nov 26, 2019
1 check passed
1 check passed
continuous-integration/jenkins/pr-merge This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.