Skip to content

Conversation

@jbaptperez
Copy link
Contributor

@jbaptperez jbaptperez commented Nov 21, 2024

IMPORTANT: All Pull Requests should be connected to an issue, if you don't
have an issue, please start by creating an issue and link it to the PR.

Please provide enough information so that others can review your pull request:

  • What existing problem does this PR solve?
  • What new feature is being introduced with this PR?
    • Add to the development set-up the possibility to customise image builds (among other) using a .env file,
    • Update of the Compose file syntax to the Compose specification,
    • Other minor enhancements related to the development Compose set-up,
    • Use a Python virtual environment for Timesketch requirements.
  • Overview of changes to existing functions if required.
    • The current behaviour remain the same, but the enhancement of the development set-up leads to different paths to the Dockefiles and simplified commands to manage the development container.

Checks

  • All tests succeed.
  • Unit tests added.
  • e2e tests added.
  • Documentation updated.

Closing issues

Closes #3225.

@google-cla
Copy link

google-cla bot commented Nov 21, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@jbaptperez jbaptperez force-pushed the compose-dev branch 2 times, most recently from f8d06cb to fb811f1 Compare November 22, 2024 10:24
@jbaptperez jbaptperez marked this pull request as ready for review November 22, 2024 10:34
@jbaptperez jbaptperez force-pushed the compose-dev branch 12 times, most recently from 62afd82 to c310703 Compare November 28, 2024 21:57
@jbaptperez
Copy link
Contributor Author

As I had time to update the PR, I also optimized some things, like using a Python virtual environment in the Docker image.
There is a lot of another improvements to be done, but maybe in another PR?

I thought about:

  • Adding an environment variable to allow a custom place for the configuration files (with a default value to be compatible), this is necessary for the next steps,
  • Put every component into its own container,
  • Using Compose profiles to start everything or only a set of services,
  • Allow, with port mapping to localhost, using our IDE to start a process in true debug mode (gunicorn or flask, celery),
  • Use Compose development features for an optimized synchronization with the current source code.

@jbaptperez jbaptperez force-pushed the compose-dev branch 2 times, most recently from 212f1bb to 08e8de9 Compare December 3, 2024 17:08
@jkppr
Copy link
Collaborator

jkppr commented Dec 12, 2024

Thanks for the suggested changes to the development setup @jbaptperez. I appreciate you taking the time to propose these updates to improve the developer experience, especially for those in restricted environments. The changes are quite extensive and touch many core parts of our development setup, so we'll need some time to thoroughly review and test them.

While flexibility is important, we aim to maintain a simple and easily reproducible development environment that works out-of-the-box on a typical Linux machine. We generally encourage developers to manage their own custom configurations for specific environments, such as those found in corporate settings. Therefore, we may incorporate some of your proposed changes while declining others to ensure we keep the core setup as straightforward as possible.

@jbaptperez
Copy link
Contributor Author

@jkppr, actually, the apparent change is not so big: Everything run into a single container with docker compose exec commands instead of docker exec ones.

I paid special attention to keeping default settings so that the behaviour without changes is the same as before.

A big change reported by Git is the deletion of the recently added yarn.lock file as it freezes a package source repository URL, which is indeed an issue for the PR.

The rest is close to the previous behaviour, with a single real difference: I duplicated the timesketch.conf file so that the development Compose set-up have its own; This simplifies the configuration from a single .env file but this is a kind of duplication, which can be discussed.

However, I understand your message and I am open to suggestions.

@jbaptperez jbaptperez force-pushed the compose-dev branch 2 times, most recently from 2161c61 to 94e3cad Compare December 23, 2024 13:48
Copy link
Collaborator

@jaegeral jaegeral left a comment

Choose a reason for hiding this comment

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

Some comments to address, I will try to have a look at the rest of the PR potentially this week.

@jbaptperez
Copy link
Contributor Author

@jaegeral, for now I keep changes in this PR untill other individual PRs are merged.
Afterwards, I'll rebase everything and already merged commits will be dropped.

@jbaptperez jbaptperez force-pushed the compose-dev branch 2 times, most recently from 4b86842 to 24981a2 Compare May 20, 2025 14:08
@jbaptperez jbaptperez force-pushed the compose-dev branch 2 times, most recently from 6d840fa to fe8bc92 Compare August 6, 2025 12:20
@jbaptperez jbaptperez force-pushed the compose-dev branch 3 times, most recently from b1dbf22 to e7afbc6 Compare August 27, 2025 14:01
@jaegeral
Copy link
Collaborator

jaegeral commented Nov 7, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request significantly enhances the development environment by overhauling the Docker Compose setup. Key improvements include the adoption of the modern Compose specification, the introduction of a .env file for easier customization of builds, and the use of a Python virtual environment within the container. The changes make the development setup more flexible and robust, especially for environments with network restrictions.

My review focuses on ensuring the new setup is correct and follows best practices. I've found a critical issue in the docker-entrypoint.sh script that would prevent the timesketch container from starting correctly due to an incorrect virtual environment path. I've also identified a couple of high-severity issues related to build reproducibility: the use of a :latest tag for a base image and ignoring package lock files in .gitignore. Finally, I've suggested some improvements to the new README.md for clarity and consistency.

@jbaptperez
Copy link
Contributor Author

jbaptperez commented Nov 9, 2025

I rebased my branch on top of master.
This led me to update my work according to the current progress:

  • Switching from Ubuntu 22.04 to Ubuntu 24.04,
  • Switching from Node 18 to Node 20 (mandatory),

Note the virtualenv became the standard way to run the application in-between, which is great.

I made the following changes:

  • Using local images built on-the-fly: They have their own directory, environment file, Dockerfile and they include the recently added health checks (with some changes),
  • Using long syntaxes in the Compose file, which helps to understand the code,
  • Changing the name of services and images, to cleary make the difference with any official images (e.g. PostgreSQL image names cannot conflict with the one for development),
  • Other minor changes.

However, I the work is not over. To do:

  • Splitting the timesketch service into individual ones:
    • Adding the development user and the sigma rules (once),
    • Starting the Celery worker,
    • Running Gunicorn (backend with auto-reload, static compiled frontend, port 5000), depends on the Celery worker,
    • Running Vite (dynamic frontend with auto-reload, port 5001, depends of Gunicorn),
  • A proper update of the README.md.

@jbaptperez jbaptperez force-pushed the compose-dev branch 4 times, most recently from 4f1abfb to 3d15cbe Compare November 12, 2025 17:14
Makes the repository handle file line endings.
This helps to make it cross-platform, asserting some files are Unix-ended.
Adds .gitignore files.
Dramatically improves an image build in a development context.
Compose standard changes are:
- Removes the deprecated "version" field,
- Adds a toplevel "name" field (prefix of container names nad network),
- Adds a toplevel "network" field, with a common "timesketch-dev"
  network,
- Removes container names (depends on and toplevel name and service
  names),
- Do not bind to the 127.0.0.1 interface only (0.0.0.0),
- Removes useless "links" (common network),
- Refactors environment variables not to use a YAML array,
- Removes "restart" fields to detect undesired crashes in development,
- Binds ports of other services to the host (opensearch, redis).

General changes are:
- Allows Docker image builds in a restricted company context (limited
  access to remote Ubuntu, Python or Node repositories) using variables,
- Centralizes variables in a .env file (not versioned),
- Adds a .env.template file as .env template with predefined variables,
- Use a distinct directory for every service dependencies,
- Use named volumes for portability and to avoid auto-creation of
  anonymous ones (PostgreSQL, Redis and Prometheus declare volumes in
  their Dockerfile; this leads to anonymous volume creations if they
  are not declared in Compose),
- Uses a per-service environment file,
- Uses local images that include their healthcheck,
- Rename some service names,
- Simplifies how development configuration files are transferred to
  Timesketch,
- Simplifies manipulation of containers using Compose CLI instead of
  the Docker one,
- Simplify and optimizes the Timesketch entrypoint,
- Updates the Bash scripts to start frontend-ng,
- Updates related documentation.
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.

Adapt the development Compose set-up to allow restricted company contexts

3 participants