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

Add image tag field to streams-bootstrap app values #499

Merged
merged 28 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 46 additions & 4 deletions docs/docs/schema/defaults.json
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,13 @@
"additionalProperties": true,
"description": "Settings specific to producers.",
"properties": {
"imageTag": {
"default": "latest",
"description": "Docker image tag of the streams-bootstrap app.",
"pattern": "^[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}$",
"title": "Imagetag",
"type": "string"
},
"nameOverride": {
"anyOf": [
{
Expand Down Expand Up @@ -1273,6 +1280,13 @@
"default": null,
"description": "Kubernetes event-driven autoscaling config"
},
"imageTag": {
"default": "latest",
"description": "Docker image tag of the streams-bootstrap app.",
"pattern": "^[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}$",
"title": "Imagetag",
"type": "string"
},
"nameOverride": {
"anyOf": [
{
Expand Down Expand Up @@ -1328,10 +1342,10 @@
"app": {
"allOf": [
{
"$ref": "#/$defs/HelmAppValues"
"$ref": "#/$defs/StreamsBootstrapValues"
}
],
"description": "Helm app values"
"description": "streams-bootstrap app values"
},
"from": {
"anyOf": [
Expand Down Expand Up @@ -1409,12 +1423,40 @@
},
"required": [
"name",
"namespace",
"app"
"namespace"
],
"title": "StreamsBootstrap",
"type": "object"
},
"StreamsBootstrapValues": {
"additionalProperties": true,
"description": "Base value class for all streams bootstrap related components.",
"properties": {
"imageTag": {
"default": "latest",
"description": "Docker image tag of the streams-bootstrap app.",
"pattern": "^[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}$",
"title": "Imagetag",
"type": "string"
},
"nameOverride": {
"anyOf": [
{
"maxLength": 63,
"type": "string"
},
{
"type": "null"
}
],
"default": null,
"description": "Helm chart name override, assigned automatically",
"title": "Nameoverride"
}
},
"title": "StreamsBootstrapValues",
"type": "object"
},
"StreamsConfig": {
"additionalProperties": true,
"description": "Streams Bootstrap streams section.",
Expand Down
14 changes: 14 additions & 0 deletions docs/docs/schema/pipeline.json
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,13 @@
"additionalProperties": true,
"description": "Settings specific to producers.",
"properties": {
"imageTag": {
"default": "latest",
"description": "Docker image tag of the streams-bootstrap app.",
"pattern": "^[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}$",
"title": "Imagetag",
"type": "string"
},
"nameOverride": {
"anyOf": [
{
Expand Down Expand Up @@ -941,6 +948,13 @@
"default": null,
"description": "Kubernetes event-driven autoscaling config"
},
"imageTag": {
"default": "latest",
"description": "Docker image tag of the streams-bootstrap app.",
"pattern": "^[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}$",
"title": "Imagetag",
"type": "string"
},
"nameOverride": {
"anyOf": [
{
Expand Down
45 changes: 44 additions & 1 deletion kpops/components/streams_bootstrap/__init__.py
Original file line number Diff line number Diff line change
@@ -1,26 +1,61 @@
from __future__ import annotations

import logging
from abc import ABC
from typing import TYPE_CHECKING

import pydantic
from pydantic import Field

from kpops.component_handlers.helm_wrapper.model import HelmRepoConfig
from kpops.components.base_components.helm_app import HelmApp
from kpops.components.base_components.helm_app import HelmApp, HelmAppValues
from kpops.utils.docstring import describe_attr

if TYPE_CHECKING:
try:
from typing import Self # pyright: ignore[reportAttributeAccessIssue]
except ImportError:
from typing_extensions import Self

STREAMS_BOOTSTRAP_HELM_REPO = HelmRepoConfig(
repository_name="bakdata-streams-bootstrap",
url="https://bakdata.github.io/streams-bootstrap/",
)
STREAMS_BOOTSTRAP_VERSION = "2.9.0"

log = logging.getLogger("StreamsBootstrap")

# Source of the pattern: https://kubernetes.io/docs/concepts/containers/images/#image-names
IMAGE_TAG_PATTERN = r"^[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}$"


class StreamsBootstrapValues(HelmAppValues):
"""Base value class for all streams bootstrap related components.

:param image_tag: Docker image tag of the streams-bootstrap app.
"""

image_tag: str = Field(
default="latest",
pattern=IMAGE_TAG_PATTERN,
description=describe_attr("image_tag", __doc__),
)


class StreamsBootstrap(HelmApp, ABC):
"""Base for components with a streams-bootstrap Helm chart.

:param app: streams-bootstrap app values
:param repo_config: Configuration of the Helm chart repo to be used for
deploying the component, defaults to streams-bootstrap Helm repo
:param version: Helm chart version, defaults to "2.9.0"
"""

app: StreamsBootstrapValues = Field(
default_factory=StreamsBootstrapValues,
description=describe_attr("app", __doc__),
)

repo_config: HelmRepoConfig = Field(
default=STREAMS_BOOTSTRAP_HELM_REPO,
description=describe_attr("repo_config", __doc__),
Expand All @@ -29,3 +64,11 @@ class StreamsBootstrap(HelmApp, ABC):
default=STREAMS_BOOTSTRAP_VERSION,
description=describe_attr("version", __doc__),
)

@pydantic.model_validator(mode="after")
def warning_for_latest_image_tag(self) -> Self:
if self.validate_ and self.app.image_tag == "latest":
log.warning(
raminqaf marked this conversation as resolved.
Show resolved Hide resolved
f"The image tag for component '{self.name}' is set or defaulted to 'latest'. Please, consider providing a stable image tag."
)
return self
3 changes: 2 additions & 1 deletion kpops/components/streams_bootstrap/producer/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
KafkaAppValues,
KafkaStreamsConfig,
)
from kpops.components.streams_bootstrap import StreamsBootstrapValues
from kpops.utils.docstring import describe_attr


class ProducerStreamsConfig(KafkaStreamsConfig):
"""Kafka Streams settings specific to Producer."""


class ProducerAppValues(KafkaAppValues):
class ProducerAppValues(StreamsBootstrapValues, KafkaAppValues):
disrupted marked this conversation as resolved.
Show resolved Hide resolved
"""Settings specific to producers.

:param streams: Kafka Streams settings
Expand Down
3 changes: 2 additions & 1 deletion kpops/components/streams_bootstrap/streams/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
KafkaStreamsConfig,
)
from kpops.components.base_components.models.topic import KafkaTopic, KafkaTopicStr
from kpops.components.streams_bootstrap import StreamsBootstrapValues
from kpops.utils.docstring import describe_attr
from kpops.utils.pydantic import (
CamelCaseConfigModel,
Expand Down Expand Up @@ -237,7 +238,7 @@ def validate_mandatory_fields_are_set(
return self


class StreamsAppValues(KafkaAppValues):
class StreamsAppValues(StreamsBootstrapValues, KafkaAppValues):
"""streams-bootstrap app configurations.

The attributes correspond to keys and values that are used as values for the streams bootstrap helm chart.
Expand Down
8 changes: 0 additions & 8 deletions tests/components/test_kafka_sink_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,6 @@ def test_connector_config_parsing(
handlers: ComponentHandlers,
connector_config: KafkaConnectorConfig,
):
connector = KafkaSinkConnector(
name=CONNECTOR_NAME,
config=config,
handlers=handlers,
app=connector_config,
resetter_namespace=RESETTER_NAMESPACE,
)

topic_pattern = ".*"
connector = KafkaSinkConnector(
name=CONNECTOR_NAME,
Expand Down
25 changes: 24 additions & 1 deletion tests/components/test_streams_bootstrap.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import re
from unittest.mock import MagicMock

import pytest
from pydantic import ValidationError
from pytest_mock import MockerFixture

from kpops.component_handlers import ComponentHandlers
Expand All @@ -10,7 +12,7 @@
HelmUpgradeInstallFlags,
)
from kpops.component_handlers.helm_wrapper.utils import create_helm_release_name
from kpops.components.streams_bootstrap import StreamsBootstrap
from kpops.components.streams_bootstrap import StreamsBootstrap, StreamsBootstrapValues
from kpops.config import KpopsConfig
from tests.components import PIPELINE_BASE_DIR

Expand Down Expand Up @@ -48,6 +50,7 @@ def test_default_configs(self, config: KpopsConfig, handlers: ComponentHandlers)
)
assert streams_bootstrap.version == "2.9.0"
assert streams_bootstrap.namespace == "test-namespace"
assert streams_bootstrap.app.image_tag == "latest"

@pytest.mark.asyncio()
async def test_should_deploy_streams_bootstrap_app(
Expand All @@ -63,6 +66,7 @@ async def test_should_deploy_streams_bootstrap_app(
**{
"namespace": "test-namespace",
"app": {
"imageTag": "1.0.0",
"streams": {
"outputTopic": "test",
"brokers": "fake-broker:9092",
Expand Down Expand Up @@ -94,10 +98,29 @@ async def test_should_deploy_streams_bootstrap_app(
"test-namespace",
{
"nameOverride": "${pipeline.name}-example-name",
"imageTag": "1.0.0",
"streams": {
"brokers": "fake-broker:9092",
"outputTopic": "test",
},
},
HelmUpgradeInstallFlags(version="1.2.3"),
)

@pytest.mark.asyncio()
async def test_should_raise_validation_error_for_invalid_image_tag(
self,
config: KpopsConfig,
handlers: ComponentHandlers,
):
with pytest.raises(
ValidationError,
match=re.escape(
"1 validation error for StreamsBootstrapValues\nimageTag\n String should match pattern '^[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}$'"
),
):
StreamsBootstrapValues(
**{
"imageTag": "invalid image tag!",
}
)
2 changes: 2 additions & 0 deletions tests/pipeline/resources/resetter_values/defaults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ helm-app:
kafka-sink-connector:
app:
"connector.class": "io.confluent.connect.jdbc.JdbcSinkConnector"
resetter_values:
imageTag: override-default-image-tag
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
- _cleaner:
app:
imageTag: latest
resources:
limits:
memory: 2G
Expand All @@ -21,6 +22,7 @@
type: producer-app-cleaner
version: 2.9.0
app:
imageTag: latest
resources:
limits:
memory: 2G
Expand Down Expand Up @@ -50,6 +52,7 @@
- _cleaner:
app:
image: some-image
imageTag: latest
labels:
pipeline: resources-custom-config
persistence:
Expand Down Expand Up @@ -77,6 +80,7 @@
version: 2.9.0
app:
image: some-image
imageTag: latest
labels:
pipeline: resources-custom-config
persistence:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
topics: []
commandLine:
CONVERT_XML: true
imageTag: latest
persistence:
enabled: false
resources:
Expand Down Expand Up @@ -105,6 +106,7 @@
topics: []
commandLine:
CONVERT_XML: true
imageTag: latest
persistence:
enabled: false
resources:
Expand Down Expand Up @@ -293,6 +295,7 @@
type: kafka-sink-connector
- _cleaner:
app:
imageTag: latest
persistence:
enabled: false
statefulSet: false
Expand All @@ -317,6 +320,7 @@
type: streams-app-cleaner
version: 2.4.2
app:
imageTag: latest
persistence:
enabled: false
statefulSet: false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
- _cleaner:
app:
image: fake-image
imageTag: latest
persistence:
enabled: false
statefulSet: false
Expand All @@ -26,6 +27,7 @@
version: 2.4.2
app:
image: fake-image
imageTag: latest
persistence:
enabled: false
statefulSet: false
Expand Down
Loading
Loading