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: add official Dockerfile for Boundary #755

Merged
merged 1 commit into from
Oct 29, 2020
Merged

feat: add official Dockerfile for Boundary #755

merged 1 commit into from
Oct 29, 2020

Conversation

malnick
Copy link
Collaborator

@malnick malnick commented Oct 28, 2020

No description provided.

@malnick malnick self-assigned this Oct 28, 2020
docker/Dockerfile Outdated Show resolved Hide resolved
samsalisbury
samsalisbury previously approved these changes Oct 28, 2020
Copy link
Contributor

@samsalisbury samsalisbury left a comment

Choose a reason for hiding this comment

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

No blockers from me, but see inline comments.

hkp://p80.pool.sks-keyservers.net:80 \
hkp://keyserver.ubuntu.com:80 \
hkp://pgp.mit.edu:80 \
; do \
Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably just hardcode the key in this repo somewhere and import it, rather than be exposed to the internet to download it, since its fingerprint is already hardcoded. Not blocking though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Totally hear that, I was torn between having consistency between our other Dockerfiles though.

scripts/uigen.sh Outdated
@@ -13,8 +13,7 @@ fi
(
cd "$UI_CLONE_DIR"
if ! docker-compose -f docker-compose-embedding.yml run build; then
echo "==> UI build failed."
exit 1
yarn install && yarn build
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't see this script referenced anywhere else in the PR, it seems like a reasonable fallback though. Note that for release builds, the UI compilation is defined here, in order to not rely on Docker. We should probably have that call this script.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I had some code that was building off HEAD and wanted this to avoid docker-in-docker. Then it turned out to be a nice fallback anyways for some other pipelines I was on.

description = "A controller for a demo!"

database {
url = "postgresql://postgres:[email protected]:5432/postgres?sslmode=disable"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Going to update this with the env:// pattern to allow easy overrides from -e flag in docker run.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

@@ -0,0 +1,46 @@
FROM alpine:3.10
Copy link
Member

Choose a reason for hiding this comment

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

All we need to do is update this to docker.mirror.hashicorp.services/alpine:3.10 to avoid the dockerhub rate limits.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated, thanks!

@malnick malnick merged commit 038540f into main Oct 29, 2020
@malnick malnick deleted the dockerfile branch October 29, 2020 15:44
@malnick malnick added this to the 0.1.2 milestone Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants