Skip to content

refactor(docker): Enhance Zebra configuration options and entrypoint logic#9344

Merged
mergify[bot] merged 10 commits intomainfrom
refactor-conf
Apr 16, 2025
Merged

refactor(docker): Enhance Zebra configuration options and entrypoint logic#9344
mergify[bot] merged 10 commits intomainfrom
refactor-conf

Conversation

@gustavovalverde
Copy link
Copy Markdown
Member

@gustavovalverde gustavovalverde commented Mar 24, 2025

Motivation

We had some issues with how we were dealing with configuration files:

  • If the user mounted a configuration file, zebra would (try to) overwrite it. This could cause odd behaviors like:
  • When mounting a configuration file using the docker compose configs this would cause an error as the mounted directory is read-only
  • We should be able to differentiate between a user-provided configuration file, our default configuration file (mainly used with docker compose), and if there's no configuration file provided, so we can build it on-the-fly with the provided variables by the user.

A possible approach to fix this was using a temporal directory, and using the resulting config from that directory instead from the original mounted dir, but this was also problematic if the temporal directory was mounted as read-only from the host, which also led to the decision of not using sed and trying to replace the content of the file or creating a new file in a location that could potentially be read-only by external host limitations.

Fixes #9325

Solution

Key changes:

  • Do not mount a default configuration file in our Dockerfile (default-zebra-config.toml), to allow more flexibility: default configuration build, default config file mounting, user-provided config file mounting with a custom $ZEBRA_CONF_PATH
    • Validate if the user is mounting its own file (this requires them to set $ZEBRA_CONF_PATH to a valid mounted file)
    • If the user doesn't set ZEBRA_CONF_PATH then validate if the default ${HOME}/.config/zebrad.toml is mounted (commonly provided with our docker compose
      # target: /home/zebra/.config/zebrad.toml
      )
    • Otherwise, create a zebra configuration file based on the variables set by the user (or using the default values)
  • Do not try to overwrite mounted files, dynamic configuration should only happen on-the-fly and must be avoided if a configuration file was provided (this changes sed to cat as using sed is no longer required)

Other (required) changes

  • exec_as_user must validate if the actual user is root, otherwise running gosu will fail when that's not the case
  • If cache or other directories were configured with the environment variables, make sure they exists and change their ownership to the user running zebrad.
  • Do not mount the .cookies directory in the same directory as the .cache as mounting a cached state would override the cookies directory.

Tests

All tests should be passing, and building with the docker compose, with our without a mount file, should give the expected configuration results.

Follow-up Work

Validate which other configurations should be added using variables

PR Checklist

  • The PR name is suitable for the release notes.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.
  • If the PR shouldn't be in the release notes, it has the
    C-exclude-from-changelog label.

@gustavovalverde gustavovalverde added C-bug Category: This is a bug A-devops Area: Pipelines, CI/CD and Dockerfiles I-integration-fail Continuous integration fails, including build and test failures I-usability Zebra is hard to understand or use P-High 🔥 labels Mar 24, 2025
gustavovalverde and others added 4 commits April 8, 2025 12:54
@gustavovalverde
Copy link
Copy Markdown
Member Author

I created #9388 to be able to test different scenarios from these changes, and made the fixes here to correctly support them.

This should be ready now @upbqdn

@gustavovalverde
Copy link
Copy Markdown
Member Author

@upbqdn I actually didn't realize I left #9388 targeting this branch. I did it just to be able to rebase this one and test the #9388. I'm planning to revert the merge, and reopen the PR, so we can merge them individually.

This is because we're going to squash the commits, and then the title and PR content will be confusing, and unrelated to most of the changes.

@gustavovalverde
Copy link
Copy Markdown
Member Author

gustavovalverde commented Apr 14, 2025

@upbqdn This PR is clean now, and ready for review/approval

@upbqdn
Copy link
Copy Markdown
Member

upbqdn commented Apr 15, 2025

@gustavovalverde
Copy link
Copy Markdown
Member Author

gustavovalverde commented Apr 16, 2025

We should update the cookie path here as well: zcashfoundation/zebra@c561064/docker/default-zebra-config.toml#L26.

This seems correct: cookie_dir = "/home/zebra/.cache/zebra"

All other locations and docs are pointing to this same dir; except for the commented out $ZEBRA_COOKIE_DIR here https://github.com/ZcashFoundation/zebra/pull/9344/files#diff-8f78f3866f02305f1df51579c939174b2c53e8295ad13bd9d2ab763176a792c1R24 which refers to a custom location, as the comment above also implies.

There are comments referring to the cookie file itself, and those are pointing to /home/zebra/.cache/zebra/.cookie

@upbqdn
Copy link
Copy Markdown
Member

upbqdn commented Apr 16, 2025

utACK

One simplification I thought of was if we could omit the first config option, and only go with the static location at /home/zebra/.config/zebrad.toml we already use in the container. The script would then check if there's a file at that location, and if not, it would generate one. Omitting custom config locations would also simplify the docs quite a bit.

@gustavovalverde
Copy link
Copy Markdown
Member Author

Omitting custom config locations would also simplify the docs quite a bit.

I agree with this, but there are edge-cases we have to consider and we might break those #3432

Unfortunately, this depend on the host and the security policies set on it for docker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-devops Area: Pipelines, CI/CD and Dockerfiles C-bug Category: This is a bug I-integration-fail Continuous integration fails, including build and test failures I-usability Zebra is hard to understand or use P-High 🔥

Projects

No open projects
Status: Won't Fix

Development

Successfully merging this pull request may close these issues.

Using custom config in Docker fails

3 participants