Use /cert less images, newer hook and some more docker-compose clean ups. #141
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Without this docker-compose complains about depends_on config settings: > ERROR: The Compose file './docker-compose.yml' is invalid because: > services.tink-server.depends_on.generate-tls-certs.condition contains "service_completed_successfully", which is an invalid type, it should be a service_started, or a service_healthy > services.web-assets-server.depends_on.fetch-and-convert-ubuntu-img.condition contains "service_completed_successfully", which is an invalid type, it should be a service_started, or a service_healthy > services.registry.depends_on.generate-tls-certs.condition contains "service_completed_successfully", which is an invalid type, it should be a service_started, or a service_healthy This was due to docker/compose#8154. Signed-off-by: Manuel Mendez <github@i.m.mmlb.dev>
It is hard to tell when setup.sh is done in vagrant without having to read/parse the output to figure out if we're at the last step. This change makes it obvious. Its not so necessary for terraform but I'd like to keep both scripts about as similar as possible. Signed-off-by: Manuel Mendez <github@i.m.mmlb.dev>
cert is going away and was never really a good one anyway. Signed-off-by: Manuel Mendez <github@i.m.mmlb.dev>
Don't need quotes for strings in yaml dicts (512M is a string due to the M) and we definitely want them whenever they appear in a shell command. Signed-off-by: Manuel Mendez <github@i.m.mmlb.dev>
To avoid confusion between the environment and cli params. Signed-off-by: Manuel Mendez <github@i.m.mmlb.dev>
Added TINKERBELL_TLS (default false) since self-signed certs are going to cause tink-worker to fail to connect. Signed-off-by: Manuel Mendez <github@i.m.mmlb.dev>
These are no longer needed (some never were) so we lets drop them. Signed-off-by: Manuel Mendez <github@i.m.mmlb.dev>
This way we can be sure that all the versions are kept in sync easily. Signed-off-by: Manuel Mendez <github@i.m.mmlb.dev>
mmlb
force-pushed
the
drop-cert
branch
2 times, most recently
from
9806515
to
b8f5b3b
Compare
jacobweinstock
approved these changes
Jun 22, 2022
Lets make .env the central/source-of-truth for env configuration. This
means we don't have to deal with empty vars and default values all over the
docker-compose.yml file. This does not cause any change in treatment of
environment variables. Actual environment variables still supersede those set
in .env. The values in .env are thus just the default if left unspecified,
exactly like ${VAR:-default} in the docker-compose.yml file.
I also got rid of container configs that used the `environment` config to
inject the env vars so that the `command` can use it. Having the values
in the environment would only really be useful if we expect to `exec`
into the container and run some commands. I haven't needed to do that yet
so would rather avoid repetition until absoulutely necessary. We can just
have docker-compose inject the values directly into the `command` line.
Signed-off-by: Manuel Mendez <github@i.m.mmlb.dev>
${} isn't actually needed in any of these uses so lets just use the shorter form.
Signed-off-by: Manuel Mendez <github@i.m.mmlb.dev>
…nk-cli to hook This way all the tink images are in sync and managed in just one place (the .env file). Signed-off-by: Manuel Mendez <github@i.m.mmlb.dev>
This has a bunch of linuxkit updates and more recent container images that have dropped /cert support. Signed-off-by: Manuel Mendez <github@i.m.mmlb.dev>
jacobweinstock
approved these changes
Jun 22, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Why is this needed
Two main things going on in this PR. First is cleaning up both the docker-compose.yml and .env files so that the docker-compose file can be written as if all the envvars it wants are always specified. Actual environment variables still supersede the values in .env like they also supersede when
${NAME:-default}is specified in the docker-compose.yml file. All in all this means we only need to specify the value of an env var with default once instead of all over the docker-compose.yml file.The other thing going on in this PR is updates to all the container images and the version of hook being used. The container images have been updated to a relatively recent version that no longer serves or fetches the grpc tls cert via the /cert http url endpoint. The version of Hook has been updated too since the previously used one is almost a year old and thus we haven't been keeping up with hook commit activity. All the versions have been properly pinned down (including tink-worker being kept in sync with tink-server and tink-cli).
How Has This Been Tested?
Both
vagrant upandterraform applyhave been run and work.