fix-doi-issues #261
fix-doi-issues #261
Conversation
…t to improve security refactor(Dockerfile): remove unused volume declarations for /liquibase/classpath and /liquibase/changelog directories in both Dockerfile and Dockerfile.alpine
|
Hey @jnewton03 @mcred could you please take a look at this PR to finish with the Docker team suggestions here docker-library/official-images#8409 (comment)? |
|
@jandroav do we have a good way to test that these are non-breaking changes? |
|
Nothing other than running liquibase commands...I think we should have something like test-harness / pro-tests to run against the docker image on every PR. |
…quibase database migration The example-changelog.xml file contains example change sets for creating tables and adding columns to an existing table. This file serves as a starting point for defining database schema changes using Liquibase. The liquibase.properties file stores properties for configuring Liquibase, including the changelog file path, target and source database URLs, usernames, and passwords. It also includes options for logging configuration and Liquibase Pro key information. The test.yml workflow file is added to run tests for Liquibase. It checks out the code, builds Docker images from Dockerfiles, runs the Trivy vulnerability scanner, starts an H2 test instance, and runs Liquibase tests using the example-changelog.xml and liquibase.properties files. These changes are made to facilitate database schema management and version control using Liquibase.
…longer needed The Trivy vulnerability scanner step was removed from the workflow as it was not providing any value and was causing unnecessary overhead.
…github/test directory in the container
…nes for cleaner file structure fix(test.yml): add command to print contents of liquibase.properties file for debugging purposes
…o access it externally
The permissions `security-events` and `actions` were removed from the build job as they are not required for the job to function properly.
…ternal instead of localhost to work with Docker networking fix(test.yml): update Docker run command to use --network=host flag to allow container to access host network
…to simplify the command and improve readability
…host-gateway to resolve host network issue fix(test.yml): remove --network host flag from docker run command to avoid potential network conflicts
…o fix connectivity issues with host.docker.internal
|
Combining all of their comments together below. If they are all covered and we've thoroughly tested the container for all use cases covered in our README.md then lets get this merged and update the PR with docker and notify them. apt-get update should usually be combined with rm -rf /var/lib/apt/lists/* so that the APT list files don't linger, since they end up getting stale quickly (and thus just add ~17MB of useless bloat to the image) addgroup and adduser should probably be done in a single layer, since they end up touching a similar set of files chown liquibase /liquibase / COPY --chown=liquibase:liquibase ... -- since /liquibase is where the actual software is installed, does it make sense for this to be owned by the unprivileged user? Typically, we see maintainers instead install the software only modifiable by root, and make just the data directory user-modifiable, which also helps prevent the runtime from modifying the application files (likely less relevant here, since this is not a long-running service but more of a oneshot tool, but still relevant IMO). RUN GNUPGHOME="$(mktemp -d)" -- having this in a line by itself doesn't actually do anything, because there's a new shell spawned for each RUN line; you'll need to either join all these RUN lines into a single RUN line, or add setting/cleanup to each (also, that value needs export if you want the gpg tool to pick it up) gpg --batch --keyserver ha.pool.sks-keyservers.net --recv-keys ... should use a full key fingerprint, not the short or long ID (for example, 79752DB6C966F0B8 becomes 0F07D1201BDDAB67CFB84EB479752DB6C966F0B8, which appears to be an expired key) gpg --batch --verify -fSLo -- this looks like a copy/pasta error from a curl invocation; I don't think you intended to pass -fSLo to gnupg we've recently updated https://github.com/docker-library/official-images#security to clarify that our ideal is both a checksum and GnuPG, since they verify different things (provenance vs transport/versioning) RUN chmod 0755 /usr/local/bin/docker-entrypoint.sh -- we've seen a lot of issues with this round-tripping through graph drivers properly due to the changed permissions; what you should do instead is commit the executable bit on the file in Git, so that what gets COPY-ed into the image can naturally be executable (Git will track the +x bit on the file properly) is there a particular reason to not put /liquibase/liquibase into the path somewhere, like a symlink in /usr/local/bin/liquibase so that users can start an interactive shell and just use liquibase directly without needing to know that it's under /liquibase? lpm update is still run |
…ntainer and mount the .github/test directory to /liquibase/changelog feat(test.yml): add step to run Liquibase version command to check the version of Liquibase being used feat(test.yml): add step to run Liquibase update command to apply the changes specified in the example-changelog.xml file
The port mapping for the H2 test instance was removed as it was not needed for the test.
…ommand to prevent blocking the workflow execution
…latest" for better compatibility and future-proofing
… improve readability
…em resource information during test execution chore(test.yml): run 'free -m' command to display memory usage during test execution
…t before running docker stats command fix(test.yml): change 'free -m' command to 'free -h' to display memory usage in human-readable format
…tarting H2 test instance chore(test.yml): add docker ps -a command to display all containers after starting H2 test instance chore(test.yml): remove free -h command for displaying memory usage before running Liquibase version
…ary image build during test workflow fix(Dockerfile): change ownership of /liquibase directory to liquibase user to avoid permission issues feat(Dockerfile): add volume mounts for /liquibase/classpath and /liquibase/changelog directories to allow for external file storage
…o use Homebrew and Colima for better compatibility and ease of use
…container before running tests
… H2 container is fully initialized before running tests
…ability and reduce noise in logs chore(Dockerfile): update base image to eclipse-temurin:17-jre-jammy to match the latest version
… allow enough time for the container to start
…faster test execution chore(test.yml): add command to display docker container status for debugging purposes
…y of the message fix(test.yml): reduce sleep time to improve efficiency of the test
…d to ensure compatibility with the target platform
…stead of 'linux/amd64' to support multiple architectures
…ing purposes The test.yml workflow file has been updated to include a new step for installing Quemu on Windows. This step is only executed when the operating system is 'windows-latest'. The purpose of this step is to install Quemu and set up the necessary dependencies for running Docker on Windows. This allows for testing the application in a Windows environment using Quemu. The step includes the following actions: - Set the execution policy to bypass and download and install Chocolatey package manager. - Install Quemu using Chocolatey. - Install WSL (Windows Subsystem for Linux) and update and install Docker on WSL. - Install Docker Machine using Chocolatey. - Create a Docker Machine using the Quemu driver and start it. - Set the DOCKER_HOST environment variable to the Docker Machine's environment. - Check the installed Docker version. This change improves the testing capabilities by providing support for running the application on Windows using Quemu.
…er-qemu fix(test.yml): remove --platform linux flag from docker build command
…mu on Windows fix(test.yml): update the installation process for Docker on Windows to use WSL and Ubuntu 20.04 feat(test.yml): add step to install Docker on Ubuntu 20.04 in WSL
The Quemu installation step for Windows has been removed as it is no longer needed for the build and test workflow.
…ensure that the container is fully initialized before checking its status and logs
…y of the test fix(test.yml): increase sleep time to allow enough time for the docker container to start
…chema feat: add mssql-jdbc-8.2.2.jre11.jar driver file for connecting to MSSQL database feat: add liquibase-mssql.properties file for configuring liquibase to connect to MSSQL database refactor: remove unused liquibase.licenseKey property from liquibase.properties file feat: add test workflow step to test driver connection to MSSQL database
…e docker container test(test.yml): improve test step by checking if the desired string is present in the logs
feat(test): add liquibase_command.sh script to handle liquibase tasks feat(test): add liquibase_update.sh script to apply changes to the database feat(test): add test workflow step to test custom entrypoint for liquibase image
The step to create a docker network using the bridge driver is removed as it is not required for the test workflow.
The step to start the mssql docker container has been removed as it is no longer needed for the test workflow.
feat(test.yml): add step to stop mssql container before starting it to ensure clean state for testing
…correctly remove the mssql container
… and add missing command to run liquibase update
…un command to improve script readability and maintainability
fix(test.yml): redirect output of docker run command to a log file for easier debugging and analysis fix(test.yml): use 'cat' command to display the contents of the log file for better visibility fix(test.yml): use 'cat' command to grep the log file for the desired string instead of running the docker command twice
…k if log contains desired string for better performance and readability
The log checking in the test workflow was unnecessary and has been removed to simplify the workflow and improve performance.
DockerfileandDockerfile.alpine