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

Docker get Ziti ID from env var instead of mounted file #510

Merged
merged 3 commits into from
Oct 10, 2022

Conversation

qrkourier
Copy link
Member

Also, start using Ziti env var names mapped to existing NF names e.g. ZITI_ENROLL_TOKEN instead of NF_REG_TOKEN while preserving backwards compat.

…t using Ziti env var names mapped to existing NF names
@qrkourier qrkourier self-assigned this Oct 6, 2022
@qrkourier qrkourier requested a review from a team as a code owner October 6, 2022 20:28
@qrkourier qrkourier linked an issue Oct 6, 2022 that may be closed by this pull request
@@ -69,7 +105,9 @@ if [[ -n "${NF_REG_NAME:-}" ]]; then
# look for enrollment token
else
echo "INFO: identity file ${IDENTITY_FILE} does not exist"
for dir in "/var/run/secrets/netfoundry.io/enrollment-token" "${IDENTITIES_DIR}"; do
for dir in "/var/run/secrets/netfoundry.io/enrollment-token" \
"/enrollment-token" \
Copy link
Member Author

Choose a reason for hiding this comment

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

This addition of /enrollment-token is for the sake of parity with the ziti-tunnel container where it was added to enable a use case where the admin wishes to supply the OTT on a separate mount point from the persisted identity. For example, this is necessary in Kubernetes where secrets and volumes may not be bound to the same mount point.

- NF_REG_NAME # inherit when run like this: NF_REG_NAME=AcmeIdentity docker-compose up ziti-tun
- NF_REG_TOKEN # NF_REG_NAME=AcmeIdentity NF_REG_TOKEN={JWT} docker-compose up ziti-tun
- PFXLOG_NO_JSON=true # suppress JSON logging
- ZITI_IDENTITY_BASENAME # inherit when run like this: ZITI_IDENTITY_BASENAME=AcmeIdentity docker-compose up ziti-tun
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how the _BASENAME suffix helps... Why not _NAME?

Copy link
Member Author

Choose a reason for hiding this comment

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

@scareything I chose _BASENAME because I have found that it is often confusing for the operator who mistakenly assumes that the correct value here is like ziti_id.json instead of the correct value ziti_id. I meant to clarify that only the basename of the file, not the full filename, is expected.

Choose a reason for hiding this comment

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

I agree, some differentiation is nice here. When I hear _NAME, I think "filename"

@qrkourier qrkourier merged commit 97aad5e into main Oct 10, 2022
@qrkourier qrkourier deleted the issue-509-docker-identity-env-var branch October 10, 2022 15:25
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.

Docker get identity from env var instead of file
3 participants