Add dev-env for containerized workflow #1276
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Yashvardhan Nanavati <[email protected]> Assisted-by: Cursor
Reviewer's GuideIntroduces a containerized development environment for the IIB worker that delegates build operations to an external Konflux cluster, including configuration, orchestration via podman-compose, and a dedicated worker configuration class driven by environment variables. Sequence diagram for containerized Konflux-based build workflowsequenceDiagram
actor Developer
participant iib_api
participant rabbitmq
participant iib_worker_containerized
participant GitLab
participant KonfluxCluster
participant IIBRegistry
participant IndexDB
Developer->>iib_api: POST /api/v1/builds/rm
iib_api-->>Developer: 202 Accepted (request_id)
iib_api->>rabbitmq: Enqueue rm build task
rabbitmq->>iib_worker_containerized: Deliver rm build task
iib_worker_containerized->>GitLab: Clone catalog repo
iib_worker_containerized->>iib_worker_containerized: Modify catalog (remove operators)
iib_worker_containerized->>GitLab: Commit and push changes
GitLab->>KonfluxCluster: Trigger PipelineRun
iib_worker_containerized->>KonfluxCluster: Watch PipelineRun status
KonfluxCluster-->>iib_worker_containerized: PipelineRun completed
iib_worker_containerized->>IIBRegistry: Copy built index image
iib_worker_containerized->>IndexDB: Update index.db artifact
iib_worker_containerized-->>rabbitmq: Mark task completed
Developer->>iib_api: GET request status
iib_api-->>Developer: Completed with built index image
Class diagram for new ContainerizedConfig worker configurationclassDiagram
class DevelopmentConfig {
}
class ContainerizedConfig {
<<extends DevelopmentConfig>>
+iib_konflux_cluster_url: Optional_str
+iib_konflux_cluster_token: Optional_str
+iib_konflux_cluster_ca_cert: Optional_str
+iib_konflux_namespace: Optional_str
+iib_konflux_pipeline_timeout: int
+iib_index_configs_gitlab_tokens_map: dict
+iib_registry: str
+iib_image_push_template: str
+iib_docker_config_template: str
+iib_index_db_artifact_registry: Optional_str
+iib_index_db_imagestream_registry: Optional_str
+iib_index_db_artifact_template: str
+include: list
+iib_log_level: str
+iib_request_logs_dir: Optional_str
+iib_aws_s3_bucket_name: Optional_str
+iib_greenwave_url: Optional_str
+iib_skopeo_timeout: str
+iib_total_attempts: int
+iib_retry_delay: int
+iib_retry_jitter: int
+iib_retry_multiplier: int
+validate() ValueError
}
DevelopmentConfig <|-- ContainerizedConfig
class CelerySettingsModule {
+module_level_config_vars
}
ContainerizedConfig ..> CelerySettingsModule: exported_as_module_level_variables
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
ContainerizedConfig, consider wrapping thejson.loadsofIIB_INDEX_CONFIGS_GITLAB_TOKENS_MAPin a try/except so that malformed JSON results in a clear configuration error rather than a generic import failure. - The containerized worker currently mounts
registry-certs-volumeat both/registry-certsand/tmp/registry-certs; if only one path is actually used, simplifying to a single mount point will make the certificate usage clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ContainerizedConfig`, consider wrapping the `json.loads` of `IIB_INDEX_CONFIGS_GITLAB_TOKENS_MAP` in a try/except so that malformed JSON results in a clear configuration error rather than a generic import failure.
- The containerized worker currently mounts `registry-certs-volume` at both `/registry-certs` and `/tmp/registry-certs`; if only one path is actually used, simplifying to a single mount point will make the certificate usage clearer.
## Individual Comments
### Comment 1
<location> `podman-compose-containerized.yml:38` </location>
<code_context>
+ - ./docker/registry/auth:/auth
+
+ db:
+ image: postgres:9.6
+ environment:
+ POSTGRES_USER: iib
</code_context>
<issue_to_address>
**🚨 issue (security):** Postgres 9.6 is EOL and represents a security/maintenance risk; consider bumping to a supported version.
Using an EOL `postgres:9.6` image increases security risk and upgrade cost. Prefer a supported major (e.g. 13–15) and update any dependent config/migrations. If you must remain on 9.6 for now, document it as an explicit exception and track an upgrade task.
</issue_to_address>
### Comment 2
<location> `podman-compose-containerized.yml:12` </location>
<code_context>
+ command:
+ - /bin/sh
+ - -c
+ - >-
+ go install github.com/jsha/minica@latest &&
+ cd /opt/app-root/certs &&
+ namei -l /opt/app-root &&
</code_context>
<issue_to_address>
**suggestion (performance):** Installing minica from source on every start with `@latest` harms reproducibility and startup time.
This approach both adds compile time to each container start and introduces non-determinism, since minica can change without any version bump here. Please either pin a specific version/tag (e.g. `@vX.Y.Z`) or use an image that already has minica installed so the registry bootstrap is deterministic and faster.
```suggestion
go install github.com/jsha/[email protected] &&
```
</issue_to_address>
### Comment 3
<location> `docker/containerized/worker_config.py:101` </location>
<code_context>
+ required_configs = {
+ 'iib_konflux_cluster_url': cls.iib_konflux_cluster_url,
+ 'iib_konflux_cluster_token': cls.iib_konflux_cluster_token,
+ 'iib_konflux_cluster_ca_cert': cls.iib_konflux_cluster_ca_cert,
+ 'iib_konflux_namespace': cls.iib_konflux_namespace,
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Validation only checks that a CA cert path string is set, not that the file actually exists or is usable.
Because `validate()` only checks for a non-empty `iib_konflux_cluster_ca_cert`, a missing `/etc/iib/konflux-ca.crt` (when the env var is unset) won’t be caught until TLS fails at runtime. Consider also verifying that the path exists (e.g., `os.path.exists(cls.iib_konflux_cluster_ca_cert)`) and is readable so misconfigurations fail fast during startup.
Suggested implementation:
```python
if missing:
raise ValueError(
f"Missing required Konflux configuration: {', '.join(missing)}. "
"Please set these in your .env.containerized file."
)
# Validate that the CA certificate path exists and is readable so we fail fast
ca_cert_path = cls.iib_konflux_cluster_ca_cert
if ca_cert_path:
import os
if not os.path.exists(ca_cert_path):
raise ValueError(
f"Konflux cluster CA certificate path does not exist: {ca_cert_path}. "
"Please ensure the path is correct or update the IIB_KONFLUX_CLUSTER_CA_CERT setting."
)
if not os.path.isfile(ca_cert_path) or not os.access(ca_cert_path, os.R_OK):
raise ValueError(
f"Konflux cluster CA certificate is not a readable file: {ca_cert_path}. "
"Please ensure the file is present and readable by the worker process."
)
```
1. If your project style disallows inline imports, move `import os` to the top of `docker/containerized/worker_config.py` with the other imports and remove it from inside `validate()`.
2. If you already have helper utilities for file validation or a centralized config validation mechanism, you may want to replace the direct `os.path` / `os.access` calls with those helpers for consistency.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| - ./docker/registry/auth:/auth | ||
|
|
||
| db: | ||
| image: postgres:9.6 |
There was a problem hiding this comment.
🚨 issue (security): Postgres 9.6 is EOL and represents a security/maintenance risk; consider bumping to a supported version.
Using an EOL postgres:9.6 image increases security risk and upgrade cost. Prefer a supported major (e.g. 13–15) and update any dependent config/migrations. If you must remain on 9.6 for now, document it as an explicit exception and track an upgrade task.
| - /bin/sh | ||
| - -c | ||
| - >- | ||
| go install github.com/jsha/minica@latest && |
There was a problem hiding this comment.
suggestion (performance): Installing minica from source on every start with @latest harms reproducibility and startup time.
This approach both adds compile time to each container start and introduces non-determinism, since minica can change without any version bump here. Please either pin a specific version/tag (e.g. @vX.Y.Z) or use an image that already has minica installed so the registry bootstrap is deterministic and faster.
Suggested change
| go install github.com/jsha/minica@latest && | |
| go install github.com/jsha/minica@v1.0.2 && |
| required_configs = { | ||
| 'iib_konflux_cluster_url': cls.iib_konflux_cluster_url, | ||
| 'iib_konflux_cluster_token': cls.iib_konflux_cluster_token, | ||
| 'iib_konflux_cluster_ca_cert': cls.iib_konflux_cluster_ca_cert, |
There was a problem hiding this comment.
suggestion (bug_risk): Validation only checks that a CA cert path string is set, not that the file actually exists or is usable.
Because validate() only checks for a non-empty iib_konflux_cluster_ca_cert, a missing /etc/iib/konflux-ca.crt (when the env var is unset) won’t be caught until TLS fails at runtime. Consider also verifying that the path exists (e.g., os.path.exists(cls.iib_konflux_cluster_ca_cert)) and is readable so misconfigurations fail fast during startup.
Suggested implementation:
if missing:
raise ValueError(
f"Missing required Konflux configuration: {', '.join(missing)}. "
"Please set these in your .env.containerized file."
)
# Validate that the CA certificate path exists and is readable so we fail fast
ca_cert_path = cls.iib_konflux_cluster_ca_cert
if ca_cert_path:
import os
if not os.path.exists(ca_cert_path):
raise ValueError(
f"Konflux cluster CA certificate path does not exist: {ca_cert_path}. "
"Please ensure the path is correct or update the IIB_KONFLUX_CLUSTER_CA_CERT setting."
)
if not os.path.isfile(ca_cert_path) or not os.access(ca_cert_path, os.R_OK):
raise ValueError(
f"Konflux cluster CA certificate is not a readable file: {ca_cert_path}. "
"Please ensure the file is present and readable by the worker process."
)- If your project style disallows inline imports, move
import osto the top ofdocker/containerized/worker_config.pywith the other imports and remove it from insidevalidate(). - If you already have helper utilities for file validation or a centralized config validation mechanism, you may want to replace the direct
os.path/os.accesscalls with those helpers for consistency.
JAVGan
approved these changes
Dec 10, 2025
JAVGan
reviewed
Dec 10, 2025
| IIB_KONFLUX_NAMESPACE=your-namespace | ||
|
|
||
| # GitLab Configuration | ||
| IIB_INDEX_CONFIGS_GITLAB_TOKENS_MAP='{"https://gitlab.example.com/catalogs/v4.19": {"token_name": "GITLAB_TOKEN_V419", "token": "glpat-xxxxxxxxxxxxx"}}' |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Yashvardhan Nanavati [email protected]
Assisted-by: Cursor
Summary by Sourcery
Introduce a containerized development environment for running IIB workers against an external Konflux cluster and associated local services.
New Features:
Documentation: