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 init container on the simplified docker compose #743

Merged
merged 3 commits into from
Jun 8, 2022

Conversation

dovholuknf
Copy link
Member

closes #742 . Also adds example how to override the env vars to produce a docker env with a totally overridden name

@dovholuknf dovholuknf requested a review from a team as a code owner June 5, 2022 17:46
if [[ "$EXTERNAL_DNS" != "" ]]; then pki_allow_list="$pki_allow_list,$EXTERNAL_DNS"; fi
pki_client_server "${pki_allow_list}" "${ZITI_CONTROLLER_INTERMEDIATE_NAME}" "${ZITI_CONTROLLER_IP_OVERRIDE-}" "${ZITI_CONTROLLER_HOSTNAME}"
pki_client_server "${ZITI_EDGE_CONTROLLER_HOSTNAME},localhost,127.0.0.1" "${ZITI_EDGE_CONTROLLER_INTERMEDIATE_NAME}" "${ZITI_EDGE_CONTROLLER_IP_OVERRIDE-}" "${ZITI_EDGE_CONTROLLER_HOSTNAME}"
pki_allow_list_dns="${ZITI_CONTROLLER_HOSTNAME},localhost,$(hostname)"
Copy link
Member

Choose a reason for hiding this comment

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

ZITI_CONTROLLER_HOSTNAME is commented out in .env. Do you mean to if check this here?

Copy link
Member Author

Choose a reason for hiding this comment

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

not for this one. i just renamed pki_allow_list to pki_allow_list_dns where it was already pki_allow_list="${ZITI_CONTROLLER_HOSTNAME},localhost,127.0.0.1" . I just kept this one the same as it was before. I did change from the IP 127.0.0.1 to $(hostname) though.

pki_allow_list_ip="127.0.0.1"
if [[ "${ZITI_EDGE_CONTROLLER_IP_OVERRIDE}" != "" ]]; then pki_allow_list_ip="${pki_allow_list_ip},${ZITI_EDGE_CONTROLLER_IP_OVERRIDE}"; fi
if [[ "${EXTERNAL_IP}" != "" ]]; then pki_allow_list_ip="${pki_allow_list_ip},${EXTERNAL_IP}"; fi

Copy link
Member

Choose a reason for hiding this comment

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

Did you cover all the possible variables (compared to .env it looks like a couple are missing if that was your intent)

Copy link
Member Author

Choose a reason for hiding this comment

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

Covered the ones I wanted to add at this time. I was hoping Geoff would be able to check to see if anything I left off is major. The variable overriding grew up organically and needs to be overhauled. I'm hoping when we have tests we'll be able to refactor this and feel good we're coving them all. @gberl002 - can you check this for me too please?

Copy link
Contributor

@gberl002 gberl002 left a comment

Choose a reason for hiding this comment

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

Tested against my WIP quickstart test and it checks out, also visually looks good and tested cli script quickstart

@dovholuknf dovholuknf merged commit ca83ae1 into release-next Jun 8, 2022
@dovholuknf dovholuknf deleted the quickstart-updates-jun5 branch June 8, 2022 15:13
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.

Add init-container to "simplified-docker-compose.yml"
3 participants