From c88df240a98a9ce36b904c2a96e0586c039c001d Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 20 Jan 2026 12:08:00 +0530 Subject: [PATCH 01/38] Support proxy values --- src/builder.py | 47 ++++++++- src/charm.py | 13 +-- src/exceptions.py | 4 - src/pipx.py | 1 - src/proxy.py | 83 ---------------- tests/conftest.py | 1 - tests/integration/conftest.py | 1 + tests/integration/test_charm.py | 1 + tests/unit/factories.py | 6 +- tests/unit/test_builder.py | 69 +++++++++++-- tests/unit/test_charm.py | 7 +- tests/unit/test_proxy.py | 166 -------------------------------- tests/unit/test_state.py | 11 +++ 13 files changed, 128 insertions(+), 282 deletions(-) delete mode 100644 src/proxy.py delete mode 100644 tests/unit/test_proxy.py diff --git a/src/builder.py b/src/builder.py index 83813f18..79501fa8 100644 --- a/src/builder.py +++ b/src/builder.py @@ -348,10 +348,10 @@ class ExternalServiceConfig: """Builder run external service dependencies. Attributes: - proxy: The proxy to use to build the image. + proxy: The proxy configuration to use to build the image. """ - proxy: str | None + proxy: state.ProxyConfig | None @dataclasses.dataclass @@ -515,7 +515,11 @@ def _run(config: RunConfig) -> list[CloudImage]: script_secrets=config.image.script_config.script_secrets, ), service_options=_ServiceOptions( - proxy=config.external_service.proxy, + proxy=( + config.external_service.proxy.http or config.external_service.proxy.https + if config.external_service.proxy + else None + ), ), ) logger.info("Run build command: %s", run_command) @@ -528,6 +532,7 @@ def _run(config: RunConfig) -> list[CloudImage]: env={ "HOME": str(UBUNTU_HOME), **_transform_secrets(secrets=config.image.script_config.script_secrets), + **_get_proxy_env(proxy=config.external_service.proxy), }, ) # The return value of the CLI is "Image build success:\n" @@ -571,6 +576,42 @@ def _transform_secrets(secrets: dict[str, str] | None) -> dict[str, str]: ) +def _get_proxy_env(proxy: state.ProxyConfig | None) -> dict[str, str]: + """Transform proxy config to standard environment variables. + + Args: + proxy: The proxy configuration. + + Returns: + Dictionary of proxy environment variables. + """ + if not proxy: + return {} + env_vars = {} + if proxy.http: + env_vars.update( + { + "http_proxy": proxy.http, + "HTTP_PROXY": proxy.http, + } + ) + if proxy.https: + env_vars.update( + { + "https_proxy": proxy.https, + "HTTPS_PROXY": proxy.https, + } + ) + if proxy.no_proxy: + env_vars.update( + { + "no_proxy": proxy.no_proxy, + "NO_PROXY": proxy.no_proxy, + } + ) + return env_vars + + @dataclasses.dataclass class _RunArgs: """Builder application run arguments. diff --git a/src/charm.py b/src/charm.py index 51c22d9d..ab637af3 100755 --- a/src/charm.py +++ b/src/charm.py @@ -4,6 +4,7 @@ # See LICENSE file for licensing details. """Entrypoint for GithubRunnerImageBuilder charm.""" + import json import logging @@ -21,7 +22,6 @@ import builder import charm_utils import image -import proxy import state LOG_FILE_DIR = Path("/var/log/github-runner-image-builder") @@ -104,7 +104,6 @@ def _on_config_changed(self, _: ops.ConfigChangedEvent) -> None: if not self._is_any_image_relation_ready(cloud_config=builder_config_state.cloud_config): return # The following lines should be covered by integration tests. - proxy.configure_aproxy(proxy=state.ProxyConfig.from_env()) # pragma: no cover builder.install_clouds_yaml( # pragma: no cover cloud_config=builder_config_state.cloud_config.openstack_clouds_config ) @@ -131,7 +130,6 @@ def _on_image_relation_changed(self, evt: ops.RelationChangedEvent) -> None: evt.unit.name, ) return - proxy.configure_aproxy(proxy=state.ProxyConfig.from_env()) builder.install_clouds_yaml( cloud_config=builder_config_state.cloud_config.openstack_clouds_config ) @@ -178,7 +176,6 @@ def _on_run_action(self, event: ops.ActionEvent) -> None: def _setup_builder(self) -> None: """Set up the builder application.""" - proxy.setup(proxy=state.ProxyConfig.from_env()) builder_config_state = state.BuilderConfig.from_charm(charm=self) builder.initialize( app_init_config=builder.ApplicationInitializationConfig( @@ -193,8 +190,7 @@ def _setup_builder(self) -> None: def _setup_logrotate(self) -> None: """Set up the log rotation for image-builder application.""" APP_LOGROTATE_CONFIG_PATH.write_text( - dedent( - f"""\ + dedent(f"""\ {str(LOG_FILE_PATH.absolute())} {{ weekly rotate 3 @@ -202,8 +198,7 @@ def _setup_logrotate(self) -> None: delaycompress missingok }} - """ - ), + """), encoding="utf-8", ) try: @@ -322,7 +317,7 @@ def _get_static_config(self, builder_config: state.BuilderConfig) -> builder.Sta runner_version=builder_config.image_config.runner_version, ), service_config=builder.ExternalServiceConfig( - proxy=(builder_config.proxy.http if builder_config.proxy else None), + proxy=builder_config.proxy, ), ) diff --git a/src/exceptions.py b/src/exceptions.py index fc0db5b2..37afbc17 100644 --- a/src/exceptions.py +++ b/src/exceptions.py @@ -24,10 +24,6 @@ class BuilderRunError(BuilderBaseError): """Represents an error while running the image builder.""" -class ProxyInstallError(BuilderBaseError): - """Represents an error while installing proxy.""" - - class GetLatestImageError(BuilderBaseError): """Represents an error while fetching the latest image.""" diff --git a/src/pipx.py b/src/pipx.py index abc9f49e..9eef91d4 100644 --- a/src/pipx.py +++ b/src/pipx.py @@ -4,7 +4,6 @@ """Module handling interactions with pipx.""" - # Code is abstracting process interactions and is currently tested in integration tests. import logging diff --git a/src/proxy.py b/src/proxy.py deleted file mode 100644 index de8b03d5..00000000 --- a/src/proxy.py +++ /dev/null @@ -1,83 +0,0 @@ -# Copyright 2025 Canonical Ltd. -# See LICENSE file for licensing details. - -"""Module for interacting with proxy.""" - -# Ignore B404:blacklist since all subprocesses are run with predefined executables. -import subprocess # nosec - -from exceptions import ProxyInstallError -from state import ProxyConfig - -UBUNTU_USER = "ubuntu" - - -def setup(proxy: ProxyConfig | None) -> None: - """Install and configure aproxy. - - Args: - proxy: The charm proxy configuration. - - Raises: - ProxyInstallError: If there was an error setting up proxy. - """ - if not proxy: - return - try: - subprocess.run( # nosec: B603 - ["/usr/bin/sudo", "snap", "install", "aproxy", "--channel=latest/edge"], - timeout=5 * 60, - check=True, - user=UBUNTU_USER, - ) - configure_aproxy(proxy=proxy) - except subprocess.SubprocessError as exc: - raise ProxyInstallError from exc - - -def configure_aproxy(proxy: ProxyConfig | None) -> None: - """Configure aproxy. - - Args: - proxy: The charm proxy configuration. - - Raises: - ProxyInstallError: If there was an error configuring aproxy. - """ - if not proxy: - return - proxy_str = (proxy.http or proxy.https).replace("http://", "").replace("https://", "") - try: - subprocess.run( # nosec: B603 - ["/usr/bin/sudo", "snap", "set", "aproxy", f"proxy={proxy_str}"], - timeout=5 * 60, - check=True, - user=UBUNTU_USER, - ) - # Ignore shell=True rule since it is safe - subprocess.run( # nosec: B602, B603 - """/usr/bin/sudo nft -f - << EOF -define default-ip = $(ip route get $(ip route show 0.0.0.0/0 \ -| grep -oP 'via \\K\\S+') | grep -oP 'src \\K\\S+') -define private-ips = { 10.0.0.0/8, 127.0.0.1/8, 172.16.0.0/12, 192.168.0.0/16 } -table ip aproxy -flush table ip aproxy -table ip aproxy { - chain prerouting { - type nat hook prerouting priority dstnat; policy accept; - ip daddr != \\$private-ips tcp dport { 80, 443 } counter dnat to \\$default-ip:8443 - } - - chain output { - type nat hook output priority -100; policy accept; - ip daddr != \\$private-ips tcp dport { 80, 443 } counter dnat to \\$default-ip:8443 - } -} -EOF""", - timeout=5 * 60, - check=True, - shell=True, - user=UBUNTU_USER, - ) - except subprocess.SubprocessError as exc: - raise ProxyInstallError("Error insttalling ") from exc diff --git a/tests/conftest.py b/tests/conftest.py index 3e7eccbc..96f5e0d2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,7 +3,6 @@ """Fixtures for github runner charm.""" - from pytest import Parser diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 2159c58d..96bc13c9 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. """Fixtures for github runner charm integration tests.""" + import functools import json import logging diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 55c42067..7a7236a9 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -4,6 +4,7 @@ # See LICENSE file for licensing details. """Integration testing module.""" + import json import logging from contextlib import asynccontextmanager diff --git a/tests/unit/factories.py b/tests/unit/factories.py index d053f22c..e7fc0b21 100644 --- a/tests/unit/factories.py +++ b/tests/unit/factories.py @@ -186,7 +186,11 @@ class Meta: # pylint: disable=too-few-public-methods model = builder.ExternalServiceConfig - proxy: str | None = "http://proxy.internal:3128" + proxy: state.ProxyConfig | None = state.ProxyConfig( + http="http://proxy.internal:3128", + https="http://proxy.internal:3128", + no_proxy="localhost,127.0.0.1", + ) class StaticConfigFactory(factory.Factory): diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index aa11513f..31729c18 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -255,9 +255,7 @@ def test_install_clouds_yaml_not_exists(monkeypatch: pytest.MonkeyPatch, tmp_pat ) contents = test_path.read_text(encoding="utf-8") - assert ( - contents - == f"""clouds: + assert contents == f"""clouds: test: auth: auth_url: test-url @@ -267,7 +265,6 @@ def test_install_clouds_yaml_not_exists(monkeypatch: pytest.MonkeyPatch, tmp_pat user_domain_name: test_domain username: test-user """ - ) def test_install_clouds_yaml_unchanged(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): @@ -300,9 +297,7 @@ def test_install_clouds_yaml_unchanged(monkeypatch: pytest.MonkeyPatch, tmp_path builder.install_clouds_yaml(cloud_config=test_config) contents = test_path.read_text(encoding="utf-8") - assert ( - contents - == f"""clouds: + assert contents == f"""clouds: test: auth: auth_url: test-url @@ -312,7 +307,6 @@ def test_install_clouds_yaml_unchanged(monkeypatch: pytest.MonkeyPatch, tmp_path user_domain_name: test_domain username: test-user """ - ) @pytest.mark.parametrize( @@ -1116,3 +1110,62 @@ def test__get_latest_image(monkeypatch: pytest.MonkeyPatch): cloud_id="test-cloud", image_id="test-image", ) + + +@pytest.mark.parametrize( + "proxy_config, expected_env", + [ + pytest.param( + None, + {}, + id="no proxy", + ), + pytest.param( + state.ProxyConfig(http="http://proxy:8080", https="", no_proxy=""), + { + "http_proxy": "http://proxy:8080", + "HTTP_PROXY": "http://proxy:8080", + }, + id="http proxy only", + ), + pytest.param( + state.ProxyConfig(http="", https="https://proxy:8080", no_proxy=""), + { + "https_proxy": "https://proxy:8080", + "HTTPS_PROXY": "https://proxy:8080", + }, + id="https proxy only", + ), + pytest.param( + state.ProxyConfig(http="", https="", no_proxy="localhost,127.0.0.1"), + { + "no_proxy": "localhost,127.0.0.1", + "NO_PROXY": "localhost,127.0.0.1", + }, + id="no_proxy only", + ), + pytest.param( + state.ProxyConfig( + http="http://proxy:8080", + https="https://proxy:8080", + no_proxy="localhost,127.0.0.1", + ), + { + "http_proxy": "http://proxy:8080", + "HTTP_PROXY": "http://proxy:8080", + "https_proxy": "https://proxy:8080", + "HTTPS_PROXY": "https://proxy:8080", + "no_proxy": "localhost,127.0.0.1", + "NO_PROXY": "localhost,127.0.0.1", + }, + id="all proxy settings", + ), + ], +) +def test__get_proxy_env(proxy_config: state.ProxyConfig | None, expected_env: dict[str, str]): + """ + arrange: given different proxy configurations. + act: when _get_proxy_env is called. + assert: expected environment variables are returned. + """ + assert builder._get_proxy_env(proxy=proxy_config) == expected_env diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index eb4eb18d..e5366d3f 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. """Unit tests for charm module.""" + import secrets # We're monkeypatching the subprocess module for testing @@ -15,7 +16,6 @@ import builder import charm as charm_module import image -import proxy import state from app.src.github_runner_image_builder.logging import LOG_FILE_PATH from charm import GithubRunnerImageBuilderCharm @@ -81,7 +81,6 @@ def test_hooks_that_trigger_run_for_all_clouds( act: when the hook is called. assert: the charm falls into ActiveStatus """ - monkeypatch.setattr(proxy, "configure_aproxy", MagicMock()) getattr(charm, hook)(MagicMock()) @@ -137,7 +136,6 @@ def test_installation( """ monkeypatch.setattr(state.BuilderConfig, "from_charm", MagicMock()) monkeypatch.setattr(image, "Observer", MagicMock()) - monkeypatch.setattr(proxy, "setup", MagicMock()) monkeypatch.setattr(builder, "initialize", (builder_setup_mock := MagicMock())) charm._setup_logrotate = (logrotate_setup_mock := MagicMock()) @@ -157,7 +155,6 @@ def test__on_image_relation_changed( act: when _on_image_relation_changed is called. assert: charm is in active status and run for the particular related unit is called. """ - monkeypatch.setattr(proxy, "configure_aproxy", MagicMock()) charm.image_observer = MagicMock() fake_clouds_auth_config = state.CloudsAuthConfig( auth_url="http://example.com", @@ -192,7 +189,6 @@ def test__on_image_relation_changed_image_already_in_cloud( act: when _on_image_relation_changed is called. assert: charm is in active status and no run is triggered but image data is updated """ - monkeypatch.setattr(proxy, "configure_aproxy", MagicMock()) charm.image_observer = MagicMock() fake_clouds_auth_config = state.CloudsAuthConfig( auth_url="http://example.com", @@ -235,7 +231,6 @@ def test__on_image_relation_changed_no_unit_auth_data( act: when _on_image_relation_changed is called. assert: charm is not building image """ - monkeypatch.setattr(proxy, "configure_aproxy", MagicMock()) charm.image_observer = MagicMock() monkeypatch.setattr( diff --git a/tests/unit/test_proxy.py b/tests/unit/test_proxy.py deleted file mode 100644 index 6a32fa3c..00000000 --- a/tests/unit/test_proxy.py +++ /dev/null @@ -1,166 +0,0 @@ -# Copyright 2025 Canonical Ltd. -# See LICENSE file for licensing details. - -"""Unit tests for proxy module.""" - -from unittest.mock import MagicMock, _Call, call - -import pytest - -import proxy -from proxy import ProxyConfig, ProxyInstallError, subprocess - - -def test_setup_error(monkeypatch: pytest.MonkeyPatch): - """ - arrange: given a monkeypatched subprocess.run that raises an error. - act: when proxy.setup is called. - assert: ProxyInstallError is raised. - """ - monkeypatch.setattr( - subprocess, - "run", - MagicMock(side_effect=subprocess.CalledProcessError(1, "", "", "Setup error")), - ) - - with pytest.raises(ProxyInstallError) as exc: - proxy.setup(MagicMock()) - - assert "Setup error" in str(exc.getrepr()) - - -def test_setup_no_proxy(monkeypatch: pytest.MonkeyPatch): - """ - arrange: given no proxy configuration. - act: when proxy.setup is called. - assert: no setup processes are called. - """ - monkeypatch.setattr(subprocess, "run", run_mock := MagicMock()) - - proxy.setup(None) - - run_mock.assert_not_called() - - -def test_setup(monkeypatch: pytest.MonkeyPatch): - """ - arrange: given a charm proxy configuration. - act: when setup is called. - assert: aproxy install command and nft table configuration command is called. - """ - monkeypatch.setattr(subprocess, "run", run_mock := MagicMock()) - monkeypatch.setattr(proxy, "configure_aproxy", configure_aproxy_mock := MagicMock()) - - proxy.setup(MagicMock()) - - configure_aproxy_mock.assert_called_once() - run_mock.assert_has_calls( - [ - call( - ["/usr/bin/sudo", "snap", "install", "aproxy", "--channel=latest/edge"], - timeout=300, - check=True, - user="ubuntu", - ) - ] - ) - - -def test_configure_aproxy_error(monkeypatch: pytest.MonkeyPatch): - """ - arrange: given monkeypatched subprocess run mocking aproxy setup. - act: when configure_aproxy is run. - assert: ProxyInstallError is raised. - """ - monkeypatch.setattr( - subprocess, - "run", - MagicMock(side_effect=subprocess.CalledProcessError(1, "", "", "Invalid proxy")), - ) - - with pytest.raises(ProxyInstallError) as exc: - proxy.configure_aproxy(MagicMock()) - - assert "Invalid proxy" in str(exc.getrepr()) - - -def test_configure_aproxy_none(monkeypatch: pytest.MonkeyPatch): - """ - arrange: given no proxy configuration. - act: when configure_aproxy is run. - assert: no setup calls are made. - """ - monkeypatch.setattr(subprocess, "run", run_mock := MagicMock()) - - proxy.configure_aproxy(None) - - run_mock.assert_not_called() - - -@pytest.mark.parametrize( - "proxy_config, expected_call", - [ - pytest.param( - ProxyConfig(http="http://proxy.internal:3128", https="", no_proxy=""), - call( - ["/usr/bin/sudo", "snap", "set", "aproxy", "proxy=proxy.internal:3128"], - timeout=300, - check=True, - user="ubuntu", - ), - id="http proxy", - ), - pytest.param( - ProxyConfig(http="", https="https://proxy.internal:3128", no_proxy=""), - call( - ["/usr/bin/sudo", "snap", "set", "aproxy", "proxy=proxy.internal:3128"], - timeout=300, - check=True, - user="ubuntu", - ), - id="https proxy", - ), - ], -) -def test_configure_aproxy( - monkeypatch: pytest.MonkeyPatch, proxy_config: ProxyConfig, expected_call: _Call -): - """ - arrange: given no proxy configuration. - act: when configure_aproxy is run. - assert: no setup calls are made. - """ - monkeypatch.setattr(subprocess, "run", run_mock := MagicMock()) - - proxy.configure_aproxy(proxy_config) - - run_mock.assert_has_calls( - [ - expected_call, - call( - """/usr/bin/sudo nft -f - << EOF -define default-ip = $(ip route get $(ip route show 0.0.0.0/0 \ -| grep -oP 'via \\K\\S+') | grep -oP 'src \\K\\S+') -define private-ips = { 10.0.0.0/8, 127.0.0.1/8, 172.16.0.0/12, 192.168.0.0/16 } -table ip aproxy -flush table ip aproxy -table ip aproxy { - chain prerouting { - type nat hook prerouting priority dstnat; policy accept; - ip daddr != \\$private-ips tcp dport { 80, 443 } counter dnat to \\$default-ip:8443 - } - - chain output { - type nat hook output priority -100; policy accept; - ip daddr != \\$private-ips tcp dport { 80, 443 } counter dnat to \\$default-ip:8443 - } -} -EOF""", - timeout=300, - check=True, - # Bandit thinks this is a real subprocess call. - shell=True, # nosec - user="ubuntu", - ), - ] - ) diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index e451ace4..ea8f388d 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -160,6 +160,17 @@ def test_proxy_config(monkeypatch: pytest.MonkeyPatch): ) +def test_proxy_config_empty(monkeypatch: pytest.MonkeyPatch): + """ + arrange: given monkeypatched os.environ with empty value. + act: when ProxyConfig.from_env is called. + assert: None is returned. + """ + monkeypatch.setattr(os, "getenv", MagicMock(return_value="")) + + assert state.ProxyConfig.from_env() is None + + @pytest.mark.parametrize( "flavor, network, expected_config", [ From 5ca542c9e3123f6378a3b99fb1a7ddcdc76c752d Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 20 Jan 2026 12:18:32 +0530 Subject: [PATCH 02/38] static errors --- src/builder.py | 2 +- src/state.py | 6 +++--- tests/unit/factories.py | 6 +++--- tests/unit/test_builder.py | 6 +++--- tests/unit/test_charm.py | 2 +- tests/unit/test_image.py | 8 ++++---- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/builder.py b/src/builder.py index 79501fa8..3e1ee2fc 100644 --- a/src/builder.py +++ b/src/builder.py @@ -52,7 +52,7 @@ OPENSTACK_CLOUDS_YAML_PATH = UBUNTU_HOME / "clouds.yaml" # Bandit thinks this is a hardcoded secret -IMAGE_BUILDER_SECRET_PREFIX = "IMAGE_BUILDER_SECRET_" # nosec: B105 +IMAGE_BUILDER_SECRET_PREFIX = "IMAGE_BUILDER_SECRET_" # nosec: hardcoded_password_string @dataclasses.dataclass diff --git a/src/state.py b/src/state.py index ef9a656f..b2cc2aeb 100644 --- a/src/state.py +++ b/src/state.py @@ -30,7 +30,7 @@ EXTERNAL_BUILD_NETWORK_CONFIG_NAME = "build-network" OPENSTACK_AUTH_URL_CONFIG_NAME = "openstack-auth-url" # Bandit thinks this is a hardcoded password -OPENSTACK_PASSWORD_CONFIG_NAME = "openstack-password" # nosec: B105 +OPENSTACK_PASSWORD_CONFIG_NAME = "openstack-password" # nosec: hardcoded_password_string OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME = "openstack-project-domain-name" OPENSTACK_PROJECT_CONFIG_NAME = "openstack-project-name" OPENSTACK_USER_DOMAIN_CONFIG_NAME = "openstack-user-domain-name" @@ -39,8 +39,8 @@ RUNNER_VERSION_CONFIG_NAME = "runner-version" SCRIPT_URL_CONFIG_NAME = "script-url" # Bandit thinks this is a hardcoded password -SCRIPT_SECRET_ID_CONFIG_NAME = "script-secret-id" # nosec: B105 -SCRIPT_SECRET_CONFIG_NAME = "script-secret" # nosec: B105 +SCRIPT_SECRET_ID_CONFIG_NAME = "script-secret-id" # nosec: hardcoded_password_string +SCRIPT_SECRET_CONFIG_NAME = "script-secret" # nosec: hardcoded_password_string IMAGE_RELATION = "image" diff --git a/tests/unit/factories.py b/tests/unit/factories.py index e7fc0b21..050965d0 100644 --- a/tests/unit/factories.py +++ b/tests/unit/factories.py @@ -103,7 +103,7 @@ class CloudAuthFactory(factory.DictFactory): auth_url = "http://testing-auth/keystone" # We need to use known password for unit testing - password = "test-password" # nosec: B105:hardcoded_password_string + password = "test-password" # nosec: hardcoded_password_string:hardcoded_password_string project_domain_name = "test-project-domain" project_name = "test-project-name" user_domain_name = "test-user-domain" @@ -174,7 +174,7 @@ class Meta: # pylint: disable=too-few-public-methods arch: state.Arch = state.Arch.ARM64 script_url: str | None = "https://test-url.com/script.sh" - script_secrets: dict[str, str] | None = {"test_secret": "test_value"} + script_secrets: dict[str, str] | None = {"test_secret": "test_value"} # nosec: hardcoded_password_string runner_version: str | None = "1.2.3" @@ -215,7 +215,7 @@ class Meta: # pylint: disable=too-few-public-methods model = builder.ScriptConfig script_url: str | None = "https://test-url.com/script.sh" - script_secrets: dict[str, str] | None = {"test_secret": "test_value"} + script_secrets: dict[str, str] | None = {"test_secret": "test_value"} # nosec: hardcoded_password_string class ImageConfigFactory(factory.Factory): diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 31729c18..cbd9f9ff 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -508,7 +508,7 @@ def test_run(monkeypatch: pytest.MonkeyPatch): prefix=TEST_STATIC_CONFIG.cloud_config.resource_prefix, script_config=builder.ScriptConfig( script_url="https://test-url.com/script.sh", - script_secrets={"test_secret": "test_value"}, + script_secrets={"test_secret": "test_value"}, # nosec: hardcoded_password_string ), runner_version="1.2.3", ), @@ -531,7 +531,7 @@ def test_run(monkeypatch: pytest.MonkeyPatch): prefix=TEST_STATIC_CONFIG.cloud_config.resource_prefix, script_config=builder.ScriptConfig( script_url="https://test-url.com/script.sh", - script_secrets={"test_secret": "test_value"}, + script_secrets={"test_secret": "test_value"}, # nosec: hardcoded_password_string ), runner_version="1.2.3", ), @@ -554,7 +554,7 @@ def test_run(monkeypatch: pytest.MonkeyPatch): prefix=TEST_STATIC_CONFIG.cloud_config.resource_prefix, script_config=builder.ScriptConfig( script_url="https://test-url.com/script.sh", - script_secrets={"test_secret": "test_value"}, + script_secrets={"test_secret": "test_value"}, # nosec: hardcoded_password_string ), runner_version="1.2.3", ), diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index e5366d3f..1b402fed 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -246,7 +246,7 @@ def test__on_image_relation_changed_no_unit_auth_data( evt.relation.data[evt.unit] = { "auth_url": "http://example.com", "username": "user", - "password": "pass", + "password": "pass", # nosec: hardcoded_password_string "project_name": "project_name", "project_domain_name": "project_domain_name", "user_domain_name": "user_domain_name", diff --git a/tests/unit/test_image.py b/tests/unit/test_image.py index e1ff5c3a..0892e5d1 100644 --- a/tests/unit/test_image.py +++ b/tests/unit/test_image.py @@ -140,7 +140,7 @@ def test_update_image_data(harness: Harness, image_observer: image.Observer): app_or_unit="github-runner/0", key_values={ "auth_url": "test", - "password": "test", + "password": "test", # nosec: hardcoded_password_string "project_domain_name": "test", "project_name": "test", "user_domain_name": "test", @@ -153,7 +153,7 @@ def test_update_image_data(harness: Harness, image_observer: image.Observer): app_or_unit="github-runner/1", key_values={ "auth_url": "test", - "password": "test", + "password": "test", # nosec: hardcoded_password_string "project_domain_name": "test", "project_name": "test", "user_domain_name": "test", @@ -172,7 +172,7 @@ def test_update_image_data(harness: Harness, image_observer: image.Observer): app_or_unit="github-runner-two/0", key_values={ "auth_url": "test", - "password": "test", + "password": "test", # nosec: hardcoded_password_string "project_domain_name": "test", "project_name": "test", "user_domain_name": "test", @@ -187,7 +187,7 @@ def test_update_image_data(harness: Harness, image_observer: image.Observer): app_or_unit="github-runner-two/1", key_values={ "auth_url": "test", - "password": "test", + "password": "test", # nosec: hardcoded_password_string "project_domain_name": "test", "project_name": "test", "user_domain_name": "test", From 209479e18a7ea3a02b87f2bb5ec09325a0a50e66 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 20 Jan 2026 12:22:52 +0530 Subject: [PATCH 03/38] fix static and lint issues --- tests/unit/factories.py | 8 ++++++-- tests/unit/test_builder.py | 12 +++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/unit/factories.py b/tests/unit/factories.py index 050965d0..9e45145e 100644 --- a/tests/unit/factories.py +++ b/tests/unit/factories.py @@ -174,7 +174,9 @@ class Meta: # pylint: disable=too-few-public-methods arch: state.Arch = state.Arch.ARM64 script_url: str | None = "https://test-url.com/script.sh" - script_secrets: dict[str, str] | None = {"test_secret": "test_value"} # nosec: hardcoded_password_string + script_secrets: dict[str, str] | None = { + "test_secret": "test_value" + } # nosec: hardcoded_password_string runner_version: str | None = "1.2.3" @@ -215,7 +217,9 @@ class Meta: # pylint: disable=too-few-public-methods model = builder.ScriptConfig script_url: str | None = "https://test-url.com/script.sh" - script_secrets: dict[str, str] | None = {"test_secret": "test_value"} # nosec: hardcoded_password_string + script_secrets: dict[str, str] | None = { + "test_secret": "test_value" + } # nosec: hardcoded_password_string class ImageConfigFactory(factory.Factory): diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index cbd9f9ff..3d3ef528 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -508,7 +508,9 @@ def test_run(monkeypatch: pytest.MonkeyPatch): prefix=TEST_STATIC_CONFIG.cloud_config.resource_prefix, script_config=builder.ScriptConfig( script_url="https://test-url.com/script.sh", - script_secrets={"test_secret": "test_value"}, # nosec: hardcoded_password_string + script_secrets={ + "test_secret": "test_value" # nosec: hardcoded_password_string + }, ), runner_version="1.2.3", ), @@ -531,7 +533,9 @@ def test_run(monkeypatch: pytest.MonkeyPatch): prefix=TEST_STATIC_CONFIG.cloud_config.resource_prefix, script_config=builder.ScriptConfig( script_url="https://test-url.com/script.sh", - script_secrets={"test_secret": "test_value"}, # nosec: hardcoded_password_string + script_secrets={ + "test_secret": "test_value", # nosec: hardcoded_password_string + }, ), runner_version="1.2.3", ), @@ -554,7 +558,9 @@ def test_run(monkeypatch: pytest.MonkeyPatch): prefix=TEST_STATIC_CONFIG.cloud_config.resource_prefix, script_config=builder.ScriptConfig( script_url="https://test-url.com/script.sh", - script_secrets={"test_secret": "test_value"}, # nosec: hardcoded_password_string + script_secrets={ + "test_secret": "test_value" # nosec: hardcoded_password_string + }, ), runner_version="1.2.3", ), From 903ce2f45b2a45d93bf2336c7cfec2ae5c3ba457 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 20 Jan 2026 12:42:12 +0530 Subject: [PATCH 04/38] fix static and lint inside app --- app/tests/conftest.py | 1 - app/tests/integration/conftest.py | 1 + app/tests/integration/helpers.py | 6 ++--- .../integration/test_openstack_builder.py | 4 ++-- app/tests/unit/test_logging.py | 1 + app/tests/unit/test_openstack_builder.py | 23 +++++++++++++++---- 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/app/tests/conftest.py b/app/tests/conftest.py index 884af93f..35cb4663 100644 --- a/app/tests/conftest.py +++ b/app/tests/conftest.py @@ -3,7 +3,6 @@ """Fixtures for github runner image builder app.""" - from pytest import Parser diff --git a/app/tests/integration/conftest.py b/app/tests/integration/conftest.py index 1c3fa53f..36ffebcc 100644 --- a/app/tests/integration/conftest.py +++ b/app/tests/integration/conftest.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. """Fixtures for github runner image builder integration tests.""" + import logging import os import secrets diff --git a/app/tests/integration/helpers.py b/app/tests/integration/helpers.py index fb12dffc..38f11a49 100644 --- a/app/tests/integration/helpers.py +++ b/app/tests/integration/helpers.py @@ -382,8 +382,7 @@ def setup_aproxy(ssh_connection: SSHConnection, proxy: str) -> None: proxy: The hostname and port in the format of "hostname:port". """ ssh_connection.run(f"/usr/bin/sudo snap set aproxy proxy={proxy} listen=:8444") - ssh_connection.run( - """/usr/bin/sudo nft -f - << EOF + ssh_connection.run("""/usr/bin/sudo nft -f - << EOF define default-ip = $(ip route get $(ip route show 0.0.0.0/0 | grep -oP 'via \\K\\S+') \ | grep -oP 'src \\K\\S+') define private-ips = { 10.0.0.0/8, 127.0.0.1/8, 172.16.0.0/12, 192.168.0.0/16 } @@ -400,8 +399,7 @@ def setup_aproxy(ssh_connection: SSHConnection, proxy: str) -> None: } } EOF -""" - ) +""") # Wait for aproxy to become active. for _ in range(6): time.sleep(5) diff --git a/app/tests/integration/test_openstack_builder.py b/app/tests/integration/test_openstack_builder.py index 23195af1..078ca204 100644 --- a/app/tests/integration/test_openstack_builder.py +++ b/app/tests/integration/test_openstack_builder.py @@ -120,8 +120,8 @@ def image_ids_fixture( script_config=config.ScriptConfig( script_url=urllib.parse.urlparse(TESTDATA_TEST_SCRIPT_URL), script_secrets={ - "TEST_SECRET": "SHOULD_EXIST", - "TEST_NON_SECRET": "SHOULD_NOT_EXIST", + "TEST_SECRET": "SHOULD_EXIST", # nosec: hardcoded_password_string + "TEST_NON_SECRET": "SHOULD_NOT_EXIST", # nosec: hardcoded_password_string }, ), ), diff --git a/app/tests/unit/test_logging.py b/app/tests/unit/test_logging.py index d6ab7efc..326904fa 100644 --- a/app/tests/unit/test_logging.py +++ b/app/tests/unit/test_logging.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. """Unit tests for logging module.""" + import logging as logging_module from pathlib import Path from unittest.mock import MagicMock diff --git a/app/tests/unit/test_openstack_builder.py b/app/tests/unit/test_openstack_builder.py index 73e71266..5b57e104 100644 --- a/app/tests/unit/test_openstack_builder.py +++ b/app/tests/unit/test_openstack_builder.py @@ -286,7 +286,11 @@ def test_run( if with_external_script else None ), - script_secrets={"TEST_SECRET_ONE": "HELLO"} if with_external_script else {}, + script_secrets=( + ({"TEST_SECRET_ONE": "HELLO"}) # nosec: hardcoded_password_string + if with_external_script + else {} + ), ), ) @@ -694,7 +698,10 @@ def test__generate_cloud_init_script( name="test-image", script_config=openstack_builder.config.ScriptConfig( script_url=urllib.parse.urlparse("https://test-url.com/script.sh"), - script_secrets={"TEST_SECRET_ONE": "HELLO", "TEST_SECRET_TWO": "WORLD"}, + script_secrets={ + "TEST_SECRET_ONE": "HELLO", # nosec: hardcoded_password_string + "TEST_SECRET_TWO": "WORLD", # nosec: hardcoded_password_string + }, ), ), proxy="test.proxy.internal:3128", @@ -948,9 +955,15 @@ def test__wait_for_cloud_init_complete(): @pytest.mark.parametrize( "script_secrets", [ - pytest.param({"TEST_SECRET_ONE": "HELLO"}, id="single secret"), pytest.param( - {"TEST_SECRET_ONE": "HELLO", "TEST_SECRET_TWO": "WORLD"}, id="multiple secrets" + {"TEST_SECRET_ONE": "HELLO"}, id="single secret" # nosec: hardcoded_password_string + ), + pytest.param( + { + "TEST_SECRET_ONE": "HELLO", # nosec: hardcoded_password_string + "TEST_SECRET_TWO": "WORLD", # nosec: hardcoded_password_string + }, + id="multiple secrets", ), pytest.param({}, id="no secrets"), ], @@ -1011,7 +1024,7 @@ def run_side_effect(*_, **__): with pytest.raises(ExternalScriptError) as exc: openstack_builder._execute_external_script( script_url="https://test-url.com/script.sh", - script_secrets={"TEST_SECRET_ONE": "HELLO"}, + script_secrets={"TEST_SECRET_ONE": "HELLO"}, # nosec: hardcoded_password_string ssh_conn=mock_connection, ) assert "Unexpected exit code" in str(exc) From 72d8d3f609dc655cf4d1c742f8761aab64ec043a Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 20 Jan 2026 15:04:12 +0530 Subject: [PATCH 05/38] abort on fail --- tests/integration/test_charm.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 7a7236a9..32665248 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -37,6 +37,7 @@ async def test_image_relation(app: Application, test_charm: Application): @pytest.mark.asyncio +@pytest.mark.abort_on_fail async def test_cos_agent_relation(app: Application): """ arrange: An active charm. @@ -55,6 +56,7 @@ async def test_cos_agent_relation(app: Application): @pytest.mark.asyncio +@pytest.mark.abort_on_fail async def test_build_image( openstack_connection: Connection, dispatch_time: datetime, @@ -70,6 +72,7 @@ async def test_build_image( # Ignore the "too many arguments" warning, as this is not significant for a test function where # the arguments are fixtures and the function is not expected to be called directly. +@pytest.mark.abort_on_fail async def test_charm_another_app_does_not_rebuild_image( # pylint: disable=R0913,R0917 app: Application, test_charm: Application, @@ -128,6 +131,7 @@ async def test_charm_another_app_does_not_rebuild_image( # pylint: disable=R091 @pytest.mark.asyncio +@pytest.mark.abort_on_fail async def test_periodic_rebuilt( app: Application, app_config: dict, @@ -179,6 +183,7 @@ async def _change_cronjob_to_minutes(unit: Unit, current_hour_interval: int): @pytest.mark.asyncio +@pytest.mark.abort_on_fail async def test_log_rotated(app: Application): """ arrange: A deployed active charm and manually write something to the log file. From 26b29653b494ddfedd63fca5d77d338580bf8955 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 20 Jan 2026 16:51:21 +0530 Subject: [PATCH 06/38] add changelog --- app/pyproject.toml | 2 +- docs/changelog.md | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/pyproject.toml b/app/pyproject.toml index 386db293..2da3b106 100644 --- a/app/pyproject.toml +++ b/app/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.12.0" +version = "0.12.1" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] diff --git a/docs/changelog.md b/docs/changelog.md index 603312f4..7151e805 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,6 +1,9 @@ +## [#185 Remove aproxy installation and add proxy support in workload](https://github.com/canonical/github-runner-image-builder-operator/pull/185) (2026-01-20) +* Remove aproxy installation in the charm and inject proxy values from the model config into the workload process. + ## [#172 feat: apt upgrade](https://github.com/canonical/github-runner-image-builder-operator/pull/172) (2025-11-26) * Apply apt-update and apt-upgrade to GH runner images by applying them during cloud-init. From 3f37f58379e5873177c180144b86bcf88459f08c Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 20 Jan 2026 16:52:42 +0530 Subject: [PATCH 07/38] vale error --- docs/changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.md b/docs/changelog.md index 7151e805..6ebb21fc 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,7 +2,7 @@ ## [#185 Remove aproxy installation and add proxy support in workload](https://github.com/canonical/github-runner-image-builder-operator/pull/185) (2026-01-20) -* Remove aproxy installation in the charm and inject proxy values from the model config into the workload process. +* Remove `aproxy` snap installation in the charm and inject proxy values from the model config into the workload process. ## [#172 feat: apt upgrade](https://github.com/canonical/github-runner-image-builder-operator/pull/172) (2025-11-26) * Apply apt-update and apt-upgrade to GH runner images by applying them during cloud-init. From 4860270277fc65b89531e64616c58e4bdc8e549c Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 27 Jan 2026 08:08:45 +0530 Subject: [PATCH 08/38] delete chain and run integ tests --- .github/workflows/integration_test.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index ebd3c341..50ac2591 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -23,6 +23,8 @@ jobs: self-hosted-runner-arch: X64 self-hosted-runner-label: pfe-ci builder-runner-label: X64 + pre-run-script: | + -c "sudo nft delete chain ip aproxy prerouting" allure-report: if: ${{ !cancelled() && github.event_name == 'schedule' }} needs: From f74433bd8d8b933ad43a028c9f9c675ac1b5bff8 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 22 Jan 2026 14:01:37 +0000 Subject: [PATCH 09/38] chore(deps): update dependency openstacksdk to v4.9.0 (#184) * chore(deps): update dependency openstacksdk to v4.9.0 * lint * fix bandit warnings * lint app * fix bandit in app * fix(tests): revert integration test secrets to hardcoded values with nosec The integration test commands.py greps for specific strings (SHOULD_EXIST, SHOULD_NOT_EXIST) to verify secret handling. Random tokens broke these checks. Co-Authored-By: Claude Opus 4.5 * fix(build): resolve PyYAML/Cython source build failure Pin wheel>=0.45 to fix incompatibility with setuptools 80.x in charmcraft's isolated build environments. Add build packages (cython3, libyaml-dev, python3-dev) needed for PyYAML source compilation. Co-Authored-By: Claude Opus 4.5 --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Christopher Bartz Co-authored-by: Claude Opus 4.5 --- app/requirements.txt | 2 +- app/tests/unit/test_openstack_builder.py | 19 ++++++------------- charmcraft.yaml | 7 +++++++ requirements.txt | 2 +- tests/unit/factories.py | 13 +++++++------ tests/unit/test_builder.py | 14 +++++--------- tests/unit/test_charm.py | 2 +- tests/unit/test_image.py | 9 +++++---- 8 files changed, 33 insertions(+), 35 deletions(-) diff --git a/app/requirements.txt b/app/requirements.txt index dcec92aa..154d506c 100644 --- a/app/requirements.txt +++ b/app/requirements.txt @@ -1,7 +1,7 @@ click==8.3.1 fabric==3.2.2 Jinja2==3.1.6 -openstacksdk==4.8.0 +openstacksdk==4.9.0 pyyaml==6.0.3 tenacity==9.1.2 typing_extensions==4.15.0 diff --git a/app/tests/unit/test_openstack_builder.py b/app/tests/unit/test_openstack_builder.py index 5b57e104..33099b40 100644 --- a/app/tests/unit/test_openstack_builder.py +++ b/app/tests/unit/test_openstack_builder.py @@ -287,9 +287,7 @@ def test_run( else None ), script_secrets=( - ({"TEST_SECRET_ONE": "HELLO"}) # nosec: hardcoded_password_string - if with_external_script - else {} + {"TEST_SECRET_ONE": secrets.token_hex(16)} if with_external_script else {} ), ), ) @@ -699,8 +697,8 @@ def test__generate_cloud_init_script( script_config=openstack_builder.config.ScriptConfig( script_url=urllib.parse.urlparse("https://test-url.com/script.sh"), script_secrets={ - "TEST_SECRET_ONE": "HELLO", # nosec: hardcoded_password_string - "TEST_SECRET_TWO": "WORLD", # nosec: hardcoded_password_string + "TEST_SECRET_ONE": secrets.token_hex(16), + "TEST_SECRET_TWO": secrets.token_hex(16), }, ), ), @@ -955,14 +953,9 @@ def test__wait_for_cloud_init_complete(): @pytest.mark.parametrize( "script_secrets", [ + pytest.param({"TEST_SECRET_ONE": secrets.token_hex(16)}, id="single secret"), pytest.param( - {"TEST_SECRET_ONE": "HELLO"}, id="single secret" # nosec: hardcoded_password_string - ), - pytest.param( - { - "TEST_SECRET_ONE": "HELLO", # nosec: hardcoded_password_string - "TEST_SECRET_TWO": "WORLD", # nosec: hardcoded_password_string - }, + {"TEST_SECRET_ONE": secrets.token_hex(16), "TEST_SECRET_TWO": secrets.token_hex(16)}, id="multiple secrets", ), pytest.param({}, id="no secrets"), @@ -1024,7 +1017,7 @@ def run_side_effect(*_, **__): with pytest.raises(ExternalScriptError) as exc: openstack_builder._execute_external_script( script_url="https://test-url.com/script.sh", - script_secrets={"TEST_SECRET_ONE": "HELLO"}, # nosec: hardcoded_password_string + script_secrets={"TEST_SECRET_ONE": secrets.token_hex(16)}, ssh_conn=mock_connection, ) assert "Unexpected exit code" in str(exc) diff --git a/charmcraft.yaml b/charmcraft.yaml index f928dd74..24f29948 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -30,6 +30,13 @@ parts: - libssl-dev # for cryptography - rust-all # for cryptography - pkg-config # for cryptography + - cython3 # for building PyYAML from source + - libyaml-dev # for PyYAML C extension + - python3-dev # for Python C extensions + charm-python-packages: + - "wheel>=0.45" + - pip + - setuptools override-build: | rustup default stable craftctl default diff --git a/requirements.txt b/requirements.txt index 57d52a9a..29518399 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ ops ==3.5.0 -openstacksdk ==4.8.0 +openstacksdk ==4.9.0 pydantic ==2.12.5 tenacity ==9.1.2 cosl ==1.4.0 diff --git a/tests/unit/factories.py b/tests/unit/factories.py index 9e45145e..3c2af34f 100644 --- a/tests/unit/factories.py +++ b/tests/unit/factories.py @@ -3,6 +3,7 @@ """Factories for generating test data.""" +import secrets import typing from unittest.mock import MagicMock @@ -174,9 +175,9 @@ class Meta: # pylint: disable=too-few-public-methods arch: state.Arch = state.Arch.ARM64 script_url: str | None = "https://test-url.com/script.sh" - script_secrets: dict[str, str] | None = { - "test_secret": "test_value" - } # nosec: hardcoded_password_string + script_secrets: dict[str, str] | None = factory.LazyFunction( + lambda: {"test_secret": secrets.token_hex(16)} + ) runner_version: str | None = "1.2.3" @@ -217,9 +218,9 @@ class Meta: # pylint: disable=too-few-public-methods model = builder.ScriptConfig script_url: str | None = "https://test-url.com/script.sh" - script_secrets: dict[str, str] | None = { - "test_secret": "test_value" - } # nosec: hardcoded_password_string + script_secrets: dict[str, str] | None = factory.LazyFunction( + lambda: {"test_secret": secrets.token_hex(16)} + ) class ImageConfigFactory(factory.Factory): diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 3d3ef528..df7db37a 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -256,6 +256,7 @@ def test_install_clouds_yaml_not_exists(monkeypatch: pytest.MonkeyPatch, tmp_pat contents = test_path.read_text(encoding="utf-8") assert contents == f"""clouds: + assert contents == f"""clouds: test: auth: auth_url: test-url @@ -298,6 +299,7 @@ def test_install_clouds_yaml_unchanged(monkeypatch: pytest.MonkeyPatch, tmp_path contents = test_path.read_text(encoding="utf-8") assert contents == f"""clouds: + assert contents == f"""clouds: test: auth: auth_url: test-url @@ -508,9 +510,7 @@ def test_run(monkeypatch: pytest.MonkeyPatch): prefix=TEST_STATIC_CONFIG.cloud_config.resource_prefix, script_config=builder.ScriptConfig( script_url="https://test-url.com/script.sh", - script_secrets={ - "test_secret": "test_value" # nosec: hardcoded_password_string - }, + script_secrets=TEST_STATIC_CONFIG.image_config.script_secrets, ), runner_version="1.2.3", ), @@ -533,9 +533,7 @@ def test_run(monkeypatch: pytest.MonkeyPatch): prefix=TEST_STATIC_CONFIG.cloud_config.resource_prefix, script_config=builder.ScriptConfig( script_url="https://test-url.com/script.sh", - script_secrets={ - "test_secret": "test_value", # nosec: hardcoded_password_string - }, + script_secrets=TEST_STATIC_CONFIG.image_config.script_secrets, ), runner_version="1.2.3", ), @@ -558,9 +556,7 @@ def test_run(monkeypatch: pytest.MonkeyPatch): prefix=TEST_STATIC_CONFIG.cloud_config.resource_prefix, script_config=builder.ScriptConfig( script_url="https://test-url.com/script.sh", - script_secrets={ - "test_secret": "test_value" # nosec: hardcoded_password_string - }, + script_secrets=TEST_STATIC_CONFIG.image_config.script_secrets, ), runner_version="1.2.3", ), diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 1b402fed..52c8c88d 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -246,7 +246,7 @@ def test__on_image_relation_changed_no_unit_auth_data( evt.relation.data[evt.unit] = { "auth_url": "http://example.com", "username": "user", - "password": "pass", # nosec: hardcoded_password_string + "password": secrets.token_hex(16), "project_name": "project_name", "project_domain_name": "project_domain_name", "user_domain_name": "user_domain_name", diff --git a/tests/unit/test_image.py b/tests/unit/test_image.py index 0892e5d1..b072da07 100644 --- a/tests/unit/test_image.py +++ b/tests/unit/test_image.py @@ -6,6 +6,7 @@ # Need access to protected functions for testing # pylint:disable=protected-access +import secrets from unittest.mock import MagicMock import pytest @@ -140,7 +141,7 @@ def test_update_image_data(harness: Harness, image_observer: image.Observer): app_or_unit="github-runner/0", key_values={ "auth_url": "test", - "password": "test", # nosec: hardcoded_password_string + "password": secrets.token_hex(16), "project_domain_name": "test", "project_name": "test", "user_domain_name": "test", @@ -153,7 +154,7 @@ def test_update_image_data(harness: Harness, image_observer: image.Observer): app_or_unit="github-runner/1", key_values={ "auth_url": "test", - "password": "test", # nosec: hardcoded_password_string + "password": secrets.token_hex(16), "project_domain_name": "test", "project_name": "test", "user_domain_name": "test", @@ -172,7 +173,7 @@ def test_update_image_data(harness: Harness, image_observer: image.Observer): app_or_unit="github-runner-two/0", key_values={ "auth_url": "test", - "password": "test", # nosec: hardcoded_password_string + "password": secrets.token_hex(16), "project_domain_name": "test", "project_name": "test", "user_domain_name": "test", @@ -187,7 +188,7 @@ def test_update_image_data(harness: Harness, image_observer: image.Observer): app_or_unit="github-runner-two/1", key_values={ "auth_url": "test", - "password": "test", # nosec: hardcoded_password_string + "password": secrets.token_hex(16), "project_domain_name": "test", "project_name": "test", "user_domain_name": "test", From 83fffe479dfc029ce2afc16f0da3aba164df85ea Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 22 Jan 2026 15:05:21 +0000 Subject: [PATCH 10/38] chore: update charm libraries (#177) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Christopher Bartz --- lib/charms/grafana_agent/v0/cos_agent.py | 47 +++++++++++++++--------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/lib/charms/grafana_agent/v0/cos_agent.py b/lib/charms/grafana_agent/v0/cos_agent.py index 7bf3eb1a..e6fc4671 100644 --- a/lib/charms/grafana_agent/v0/cos_agent.py +++ b/lib/charms/grafana_agent/v0/cos_agent.py @@ -254,7 +254,7 @@ class _MetricsEndpointDict(TypedDict): LIBID = "dc15fa84cef84ce58155fb84f6c6213a" LIBAPI = 0 -LIBPATCH = 22 +LIBPATCH = 23 PYDEPS = ["cosl >= 0.0.50", "pydantic"] @@ -264,12 +264,6 @@ class _MetricsEndpointDict(TypedDict): logger = logging.getLogger(__name__) SnapEndpoint = namedtuple("SnapEndpoint", "owner, name") -# Note: MutableMapping is imported from the typing module and not collections.abc -# because subscripting collections.abc.MutableMapping was added in python 3.9, but -# most of our charms are based on 20.04, which has python 3.8. - -_RawDatabag = MutableMapping[str, str] - class TransportProtocolType(str, enum.Enum): """Receiver Type.""" @@ -305,6 +299,15 @@ class TransportProtocolType(str, enum.Enum): ReceiverProtocol = Literal["otlp_grpc", "otlp_http", "zipkin", "jaeger_thrift_http", "jaeger_grpc"] +def _dedupe_list(items: List[Dict[str, Any]]) -> List[Dict[str, Any]]: + """Deduplicate items in the list via object identity.""" + unique_items = [] + for item in items: + if item not in unique_items: + unique_items.append(item) + return unique_items + + class TracingError(Exception): """Base class for custom errors raised by tracing.""" @@ -619,7 +622,8 @@ def __init__( refresh_events: Optional[List] = None, tracing_protocols: Optional[List[str]] = None, *, - scrape_configs: Optional[Union[List[dict], Callable]] = None, + scrape_configs: Optional[Union[List[dict], Callable[[], List[Dict[str, Any]]]]] = None, + extra_alert_groups: Optional[Callable[[], Dict[str, Any]]] = None, ): """Create a COSAgentProvider instance. @@ -640,6 +644,9 @@ def __init__( scrape_configs: List of standard scrape_configs dicts or a callable that returns the list in case the configs need to be generated dynamically. The contents of this list will be merged with the contents of `metrics_endpoints`. + extra_alert_groups: A callable that returns a dict of alert rule groups in case the + alerts need to be generated dynamically. The contents of this dict will be merged + with generic and bundled alert rules. """ super().__init__(charm, relation_name) dashboard_dirs = dashboard_dirs or ["./src/grafana_dashboards"] @@ -648,6 +655,7 @@ def __init__( self._relation_name = relation_name self._metrics_endpoints = metrics_endpoints or [] self._scrape_configs = scrape_configs or [] + self._extra_alert_groups = extra_alert_groups or {} self._metrics_rules = metrics_rules_dir self._logs_rules = logs_rules_dir self._recursive = recurse_rules_dirs @@ -691,10 +699,11 @@ def _on_refresh(self, event): @property def _scrape_jobs(self) -> List[Dict]: - """Return a prometheus_scrape-like data structure for jobs. + """Return a list of scrape_configs. https://prometheus.io/docs/prometheus/latest/configuration/configuration/#scrape_config """ + # Optionally allow the charm to set the scrape_configs if callable(self._scrape_configs): scrape_configs = self._scrape_configs() else: @@ -712,17 +721,17 @@ def _scrape_jobs(self) -> List[Dict]: scrape_configs = scrape_configs or [] - # Augment job name to include the app name and a unique id (index) - for idx, scrape_config in enumerate(scrape_configs): - scrape_config["job_name"] = "_".join( - [self._charm.app.name, str(idx), scrape_config.get("job_name", "default")] - ) - return scrape_configs @property def _metrics_alert_rules(self) -> Dict: - """Use (for now) the prometheus_scrape AlertRules to initialize this.""" + """Return a dict of alert rule groups.""" + # Optionally allow the charm to add the metrics_alert_rules + if callable(self._extra_alert_groups): + rules = self._extra_alert_groups() + else: + rules = {"groups": []} + alert_rules = AlertRules( query_type="promql", topology=JujuTopology.from_charm(self._charm) ) @@ -731,7 +740,11 @@ def _metrics_alert_rules(self) -> Dict: generic_alert_groups.application_rules, group_name_prefix=JujuTopology.from_charm(self._charm).identifier, ) - return alert_rules.as_dict() + + # NOTE: The charm could supply rules we implement in this method, so we deduplicate + rules["groups"] = _dedupe_list(rules["groups"] + alert_rules.as_dict()["groups"]) + + return rules @property def _log_alert_rules(self) -> Dict: From e49a597816925ac22a09416e5397c7c6691663ba Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 27 Jan 2026 08:23:38 +0530 Subject: [PATCH 11/38] Update src/builder.py Co-authored-by: Christopher Bartz --- src/builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/builder.py b/src/builder.py index 3e1ee2fc..d24093de 100644 --- a/src/builder.py +++ b/src/builder.py @@ -577,7 +577,7 @@ def _transform_secrets(secrets: dict[str, str] | None) -> dict[str, str]: def _get_proxy_env(proxy: state.ProxyConfig | None) -> dict[str, str]: - """Transform proxy config to standard environment variables. + """Transform proxy config to dict of standard environment variables. Args: proxy: The proxy configuration. From d901d07d73e78be2ae5d52f2dd5e556d053c75ef Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 27 Jan 2026 08:24:58 +0530 Subject: [PATCH 12/38] remove pyproject update --- app/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/pyproject.toml b/app/pyproject.toml index 2da3b106..386db293 100644 --- a/app/pyproject.toml +++ b/app/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.12.1" +version = "0.12.0" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] From ad07dc9071938ffef37b1137dd411efbabd58939 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Thu, 5 Feb 2026 14:08:09 +0530 Subject: [PATCH 13/38] address code review --- .github/workflows/integration_test.yaml | 2 -- src/charm.py | 18 +++++++++++++ tests/unit/factories.py | 2 +- tests/unit/test_charm.py | 34 ++++++++++++++++++++++++- 4 files changed, 52 insertions(+), 4 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 50ac2591..ebd3c341 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -23,8 +23,6 @@ jobs: self-hosted-runner-arch: X64 self-hosted-runner-label: pfe-ci builder-runner-label: X64 - pre-run-script: | - -c "sudo nft delete chain ip aproxy prerouting" allure-report: if: ${{ !cancelled() && github.event_name == 'schedule' }} needs: diff --git a/src/charm.py b/src/charm.py index ab637af3..be27b518 100755 --- a/src/charm.py +++ b/src/charm.py @@ -7,6 +7,7 @@ import json import logging +import os # We ignore low severity security warning for importing subprocess module import subprocess # nosec B404 @@ -174,9 +175,26 @@ def _on_run_action(self, event: ops.ActionEvent) -> None: # The following line should be covered by the integration test. self._run() # pragma: nocover + def _setup_proxy_environment(self, proxy_config: state.ProxyConfig | None) -> None: + """Set up proxy environment variables. + + Args: + proxy_config: The proxy configuration to apply to environment variables. + """ + if proxy_config: + os.environ["http_proxy"] = proxy_config.http + os.environ["https_proxy"] = proxy_config.https + os.environ["no_proxy"] = proxy_config.no_proxy + os.environ["HTTP_PROXY"] = proxy_config.http + os.environ["HTTPS_PROXY"] = proxy_config.https + os.environ["NO_PROXY"] = proxy_config.no_proxy + def _setup_builder(self) -> None: """Set up the builder application.""" builder_config_state = state.BuilderConfig.from_charm(charm=self) + + self._setup_proxy_environment(builder_config_state.proxy) + builder.initialize( app_init_config=builder.ApplicationInitializationConfig( cloud_config=builder_config_state.cloud_config, diff --git a/tests/unit/factories.py b/tests/unit/factories.py index 3c2af34f..934d5c2a 100644 --- a/tests/unit/factories.py +++ b/tests/unit/factories.py @@ -104,7 +104,7 @@ class CloudAuthFactory(factory.DictFactory): auth_url = "http://testing-auth/keystone" # We need to use known password for unit testing - password = "test-password" # nosec: hardcoded_password_string:hardcoded_password_string + password = "test-password" # nosec: hardcoded_password_string project_domain_name = "test-project-domain" project_name = "test-project-name" user_domain_name = "test-user-domain" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 52c8c88d..de96d77a 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -3,6 +3,7 @@ """Unit tests for charm module.""" +import os import secrets # We're monkeypatching the subprocess module for testing @@ -134,7 +135,11 @@ def test_installation( act: when _on_install is called. assert: setup_builder is called. """ - monkeypatch.setattr(state.BuilderConfig, "from_charm", MagicMock()) + builder_config_mock = MagicMock() + builder_config_mock.proxy = None + monkeypatch.setattr( + state.BuilderConfig, "from_charm", MagicMock(return_value=builder_config_mock) + ) monkeypatch.setattr(image, "Observer", MagicMock()) monkeypatch.setattr(builder, "initialize", (builder_setup_mock := MagicMock())) charm._setup_logrotate = (logrotate_setup_mock := MagicMock()) @@ -303,3 +308,30 @@ def test__setup_logrotate(monkeypatch, tmp_path, charm: GithubRunnerImageBuilder mock_check_call.assert_called_once_with( ["/usr/sbin/logrotate", str(logrotate_path), "--debug"] ) + + +def test_setup_proxy_environment_with_proxy_config( + monkeypatch: pytest.MonkeyPatch, charm: GithubRunnerImageBuilderCharm +): + """ + arrange: given a ProxyConfig with http, https, and no_proxy values. + act: when _setup_proxy_environment is called. + assert: environment variables are set correctly. + """ + for key in ["http_proxy", "https_proxy", "no_proxy", "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"]: + monkeypatch.delenv(key, raising=False) + + proxy_config = state.ProxyConfig( + http="http://proxy.example.com:8080", + https="https://proxy.example.com:8443", + no_proxy="localhost,127.0.0.1", + ) + + charm._setup_proxy_environment(proxy_config) + + assert os.environ["http_proxy"] == "http://proxy.example.com:8080" + assert os.environ["https_proxy"] == "https://proxy.example.com:8443" + assert os.environ["no_proxy"] == "localhost,127.0.0.1" + assert os.environ["HTTP_PROXY"] == "http://proxy.example.com:8080" + assert os.environ["HTTPS_PROXY"] == "https://proxy.example.com:8443" + assert os.environ["NO_PROXY"] == "localhost,127.0.0.1" From 7cb4485ff368b5b66d04aa6d6ed4320a6dde4141 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Thu, 5 Feb 2026 14:10:08 +0530 Subject: [PATCH 14/38] remove tesT --- tests/unit/test_state.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index ea8f388d..e451ace4 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -160,17 +160,6 @@ def test_proxy_config(monkeypatch: pytest.MonkeyPatch): ) -def test_proxy_config_empty(monkeypatch: pytest.MonkeyPatch): - """ - arrange: given monkeypatched os.environ with empty value. - act: when ProxyConfig.from_env is called. - assert: None is returned. - """ - monkeypatch.setattr(os, "getenv", MagicMock(return_value="")) - - assert state.ProxyConfig.from_env() is None - - @pytest.mark.parametrize( "flavor, network, expected_config", [ From 1d7a529092197a53ddb8f268be2c098d1fac9482 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Fri, 6 Feb 2026 11:37:49 +0530 Subject: [PATCH 15/38] test --- .github/workflows/integration_test.yaml | 2 ++ tests/integration/conftest.py | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index ebd3c341..50ac2591 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -23,6 +23,8 @@ jobs: self-hosted-runner-arch: X64 self-hosted-runner-label: pfe-ci builder-runner-label: X64 + pre-run-script: | + -c "sudo nft delete chain ip aproxy prerouting" allure-report: if: ${{ !cancelled() && github.event_name == 'schedule' }} needs: diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 96bc13c9..c2ae0094 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -106,6 +106,10 @@ async def model_fixture(proxy: ProxyConfig, ops_test: OpsTest) -> AsyncGenerator "juju-http-proxy": proxy.http, "juju-https-proxy": proxy.https, "juju-no-proxy": proxy.no_proxy, + "apt-http-proxy": proxy.http, + "apt-https-proxy": proxy.https, + "snap-http-proxy": proxy.http, + "snap-https-proxy": proxy.https, } ) yield ops_test.model From 7ef5451426cbf24578dc40b536a9b03fc117b6f5 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 11:37:46 +0530 Subject: [PATCH 16/38] update prerouting chain --- .github/workflows/integration_test.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 50ac2591..b9da758a 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -24,7 +24,10 @@ jobs: self-hosted-runner-label: pfe-ci builder-runner-label: X64 pre-run-script: | - -c "sudo nft delete chain ip aproxy prerouting" + -c "lxc list + IP=$(lxc list -c 4 --format csv | awk '{print $1}' | head -n 1) + echo '$IP' + sudo nft insert rule ip aproxy prerouting index 0 ip saddr != $IP return" allure-report: if: ${{ !cancelled() && github.event_name == 'schedule' }} needs: From 417b9efd6aeb655bd9a7044fd1b6b81c0819d19f Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 11:51:41 +0530 Subject: [PATCH 17/38] fix script --- .github/workflows/integration_test.yaml | 6 ++---- tests/integration/pre_run_script.sh | 11 +++++++++++ 2 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 tests/integration/pre_run_script.sh diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index b9da758a..aba5628e 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -24,10 +24,8 @@ jobs: self-hosted-runner-label: pfe-ci builder-runner-label: X64 pre-run-script: | - -c "lxc list - IP=$(lxc list -c 4 --format csv | awk '{print $1}' | head -n 1) - echo '$IP' - sudo nft insert rule ip aproxy prerouting index 0 ip saddr != $IP return" + -c "chmod +x tests/integration/pre_run_script.sh + ./tests/integration/pre_run_script.sh" allure-report: if: ${{ !cancelled() && github.event_name == 'schedule' }} needs: diff --git a/tests/integration/pre_run_script.sh b/tests/integration/pre_run_script.sh new file mode 100644 index 00000000..70db5fa4 --- /dev/null +++ b/tests/integration/pre_run_script.sh @@ -0,0 +1,11 @@ +#!/bin/bash + +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +set -euo pipefail + +lxc list +IP=$(lxc list -c 4 --format csv | awk '{print $1}' | head -n 1) +echo "IP=$IP" +sudo nft insert rule ip aproxy prerouting index 0 ip saddr != $IP return From 08d10b44d1abc91ef0ab832130dcc71eaed349e1 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 11:52:34 +0530 Subject: [PATCH 18/38] fix script --- tests/integration/pre_run_script.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/pre_run_script.sh b/tests/integration/pre_run_script.sh index 70db5fa4..d3e8d017 100644 --- a/tests/integration/pre_run_script.sh +++ b/tests/integration/pre_run_script.sh @@ -8,4 +8,4 @@ set -euo pipefail lxc list IP=$(lxc list -c 4 --format csv | awk '{print $1}' | head -n 1) echo "IP=$IP" -sudo nft insert rule ip aproxy prerouting index 0 ip saddr != $IP return +sudo nft insert rule ip aproxy prerouting index 0 ip saddr != "$IP" return From 1e68b2a5c8a06b07da8cce0007e8b406b9c61d38 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 12:45:41 +0530 Subject: [PATCH 19/38] update builder initializer --- src/builder.py | 19 +++++++++++++++++-- src/charm.py | 1 + tests/unit/test_builder.py | 6 +++++- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/builder.py b/src/builder.py index d24093de..ed4ab6c2 100644 --- a/src/builder.py +++ b/src/builder.py @@ -65,6 +65,7 @@ class ApplicationInitializationConfig: image_arch: The image architecture to initialize build resources for. resource_prefix: The prefix of application resources. unit_name: The Juju unit name to trigger the CRON with. + proxy: The http(s) proxy configuration. """ cloud_config: state.CloudConfig @@ -72,6 +73,7 @@ class ApplicationInitializationConfig: image_arch: state.Arch resource_prefix: str unit_name: str + proxy: state.ProxyConfig | None def initialize(app_init_config: ApplicationInitializationConfig) -> None: @@ -93,6 +95,7 @@ def initialize(app_init_config: ApplicationInitializationConfig) -> None: cloud_name=app_init_config.cloud_config.cloud_name, image_arch=app_init_config.image_arch, resource_prefix=app_init_config.resource_prefix, + proxy_config=app_init_config.proxy, ) configure_cron( unit_name=app_init_config.unit_name, interval=app_init_config.cron_interval @@ -137,7 +140,10 @@ def _install_app() -> None: def _initialize_image_builder( - cloud_name: str, image_arch: state.Arch, resource_prefix: str + cloud_name: str, + image_arch: state.Arch, + resource_prefix: str, + proxy_config: state.ProxyConfig | None, ) -> None: """Initialize github-runner-image-builder app. @@ -145,6 +151,7 @@ def _initialize_image_builder( cloud_name: The OpenStack cloud to pre-populate OpenStack image builder resources. image_arch: The architecture of the image to build. resource_prefix: The resource prefix for artefacts saved in the image repository. + proxy_config: The proxy configuration to apply to the environment. Raises: ImageBuilderInitializeError: If there was an error Initialize the app. @@ -152,6 +159,14 @@ def _initialize_image_builder( init_cmd = _build_init_command( cloud_name=cloud_name, image_arch=image_arch, resource_prefix=resource_prefix ) + env = os.environ.copy() + if proxy_config: + env["http_proxy"] = proxy_config.http + env["https_proxy"] = proxy_config.https + env["no_proxy"] = proxy_config.no_proxy + env["HTTP_PROXY"] = proxy_config.http + env["HTTPS_PROXY"] = proxy_config.https + env["NO_PROXY"] = proxy_config.no_proxy try: subprocess.run( init_cmd, @@ -159,7 +174,7 @@ def _initialize_image_builder( user=UBUNTU_USER, cwd=UBUNTU_HOME, timeout=15 * 60, - env=os.environ, + env=env, ) # nosec: B603 except subprocess.CalledProcessError as exc: logger.error( diff --git a/src/charm.py b/src/charm.py index be27b518..d375b72b 100755 --- a/src/charm.py +++ b/src/charm.py @@ -202,6 +202,7 @@ def _setup_builder(self) -> None: image_arch=builder_config_state.image_config.arch, resource_prefix=builder_config_state.app_config.resource_prefix, unit_name=self.unit.name, + proxy=builder_config_state.proxy, ) ) diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index a17fe4ce..8b1083db 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -67,6 +67,7 @@ def test_initialize(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): image_arch=MagicMock(), resource_prefix=MagicMock(), unit_name=MagicMock(), + proxy=None, ) ) @@ -175,7 +176,10 @@ def test__initialize_image_builder_error( with pytest.raises(builder.ImageBuilderInitializeError): builder._initialize_image_builder( - cloud_name=MagicMock(), image_arch=MagicMock(), resource_prefix=MagicMock() + cloud_name=MagicMock(), + image_arch=MagicMock(), + resource_prefix=MagicMock(), + proxy_config=None, ) From efbadff6b56461fca9e34e71c49c2266fbd7ece1 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 14:50:01 +0530 Subject: [PATCH 20/38] remove all code --- src/builder.py | 59 ++++---------------------------------- src/charm.py | 2 +- tests/unit/factories.py | 6 +--- tests/unit/test_builder.py | 59 -------------------------------------- 4 files changed, 8 insertions(+), 118 deletions(-) diff --git a/src/builder.py b/src/builder.py index ed4ab6c2..01390789 100644 --- a/src/builder.py +++ b/src/builder.py @@ -159,14 +159,7 @@ def _initialize_image_builder( init_cmd = _build_init_command( cloud_name=cloud_name, image_arch=image_arch, resource_prefix=resource_prefix ) - env = os.environ.copy() - if proxy_config: - env["http_proxy"] = proxy_config.http - env["https_proxy"] = proxy_config.https - env["no_proxy"] = proxy_config.no_proxy - env["HTTP_PROXY"] = proxy_config.http - env["HTTPS_PROXY"] = proxy_config.https - env["NO_PROXY"] = proxy_config.no_proxy + logger.info("Initialize image builder with env: %s", os.environ) try: subprocess.run( init_cmd, @@ -174,7 +167,7 @@ def _initialize_image_builder( user=UBUNTU_USER, cwd=UBUNTU_HOME, timeout=15 * 60, - env=env, + env=os.environ, ) # nosec: B603 except subprocess.CalledProcessError as exc: logger.error( @@ -363,10 +356,10 @@ class ExternalServiceConfig: """Builder run external service dependencies. Attributes: - proxy: The proxy configuration to use to build the image. + proxy: The proxy to use to build the image. """ - proxy: state.ProxyConfig | None + proxy: str | None @dataclasses.dataclass @@ -530,11 +523,7 @@ def _run(config: RunConfig) -> list[CloudImage]: script_secrets=config.image.script_config.script_secrets, ), service_options=_ServiceOptions( - proxy=( - config.external_service.proxy.http or config.external_service.proxy.https - if config.external_service.proxy - else None - ), + proxy=config.external_service.proxy, ), ) logger.info("Run build command: %s", run_command) @@ -547,7 +536,7 @@ def _run(config: RunConfig) -> list[CloudImage]: env={ "HOME": str(UBUNTU_HOME), **_transform_secrets(secrets=config.image.script_config.script_secrets), - **_get_proxy_env(proxy=config.external_service.proxy), + **os.environ, }, ) # The return value of the CLI is "Image build success:\n" @@ -591,42 +580,6 @@ def _transform_secrets(secrets: dict[str, str] | None) -> dict[str, str]: ) -def _get_proxy_env(proxy: state.ProxyConfig | None) -> dict[str, str]: - """Transform proxy config to dict of standard environment variables. - - Args: - proxy: The proxy configuration. - - Returns: - Dictionary of proxy environment variables. - """ - if not proxy: - return {} - env_vars = {} - if proxy.http: - env_vars.update( - { - "http_proxy": proxy.http, - "HTTP_PROXY": proxy.http, - } - ) - if proxy.https: - env_vars.update( - { - "https_proxy": proxy.https, - "HTTPS_PROXY": proxy.https, - } - ) - if proxy.no_proxy: - env_vars.update( - { - "no_proxy": proxy.no_proxy, - "NO_PROXY": proxy.no_proxy, - } - ) - return env_vars - - @dataclasses.dataclass class _RunArgs: """Builder application run arguments. diff --git a/src/charm.py b/src/charm.py index d375b72b..53282a75 100755 --- a/src/charm.py +++ b/src/charm.py @@ -336,7 +336,7 @@ def _get_static_config(self, builder_config: state.BuilderConfig) -> builder.Sta runner_version=builder_config.image_config.runner_version, ), service_config=builder.ExternalServiceConfig( - proxy=builder_config.proxy, + proxy=(builder_config.proxy.http if builder_config.proxy else None), ), ) diff --git a/tests/unit/factories.py b/tests/unit/factories.py index 934d5c2a..153480a0 100644 --- a/tests/unit/factories.py +++ b/tests/unit/factories.py @@ -189,11 +189,7 @@ class Meta: # pylint: disable=too-few-public-methods model = builder.ExternalServiceConfig - proxy: state.ProxyConfig | None = state.ProxyConfig( - http="http://proxy.internal:3128", - https="http://proxy.internal:3128", - no_proxy="localhost,127.0.0.1", - ) + proxy: str | None = "http://proxy.internal:3128" class StaticConfigFactory(factory.Factory): diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 8b1083db..f731d691 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -1114,62 +1114,3 @@ def test__get_latest_image(monkeypatch: pytest.MonkeyPatch): cloud_id="test-cloud", image_id="test-image", ) - - -@pytest.mark.parametrize( - "proxy_config, expected_env", - [ - pytest.param( - None, - {}, - id="no proxy", - ), - pytest.param( - state.ProxyConfig(http="http://proxy:8080", https="", no_proxy=""), - { - "http_proxy": "http://proxy:8080", - "HTTP_PROXY": "http://proxy:8080", - }, - id="http proxy only", - ), - pytest.param( - state.ProxyConfig(http="", https="https://proxy:8080", no_proxy=""), - { - "https_proxy": "https://proxy:8080", - "HTTPS_PROXY": "https://proxy:8080", - }, - id="https proxy only", - ), - pytest.param( - state.ProxyConfig(http="", https="", no_proxy="localhost,127.0.0.1"), - { - "no_proxy": "localhost,127.0.0.1", - "NO_PROXY": "localhost,127.0.0.1", - }, - id="no_proxy only", - ), - pytest.param( - state.ProxyConfig( - http="http://proxy:8080", - https="https://proxy:8080", - no_proxy="localhost,127.0.0.1", - ), - { - "http_proxy": "http://proxy:8080", - "HTTP_PROXY": "http://proxy:8080", - "https_proxy": "https://proxy:8080", - "HTTPS_PROXY": "https://proxy:8080", - "no_proxy": "localhost,127.0.0.1", - "NO_PROXY": "localhost,127.0.0.1", - }, - id="all proxy settings", - ), - ], -) -def test__get_proxy_env(proxy_config: state.ProxyConfig | None, expected_env: dict[str, str]): - """ - arrange: given different proxy configurations. - act: when _get_proxy_env is called. - assert: expected environment variables are returned. - """ - assert builder._get_proxy_env(proxy=proxy_config) == expected_env From a89fbdbf5b01c400eb7ce90cc34682cff1caf323 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 14:55:53 +0530 Subject: [PATCH 21/38] remove code --- src/builder.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/builder.py b/src/builder.py index 01390789..3bf47cde 100644 --- a/src/builder.py +++ b/src/builder.py @@ -65,7 +65,6 @@ class ApplicationInitializationConfig: image_arch: The image architecture to initialize build resources for. resource_prefix: The prefix of application resources. unit_name: The Juju unit name to trigger the CRON with. - proxy: The http(s) proxy configuration. """ cloud_config: state.CloudConfig @@ -73,7 +72,6 @@ class ApplicationInitializationConfig: image_arch: state.Arch resource_prefix: str unit_name: str - proxy: state.ProxyConfig | None def initialize(app_init_config: ApplicationInitializationConfig) -> None: @@ -95,7 +93,6 @@ def initialize(app_init_config: ApplicationInitializationConfig) -> None: cloud_name=app_init_config.cloud_config.cloud_name, image_arch=app_init_config.image_arch, resource_prefix=app_init_config.resource_prefix, - proxy_config=app_init_config.proxy, ) configure_cron( unit_name=app_init_config.unit_name, interval=app_init_config.cron_interval @@ -140,10 +137,7 @@ def _install_app() -> None: def _initialize_image_builder( - cloud_name: str, - image_arch: state.Arch, - resource_prefix: str, - proxy_config: state.ProxyConfig | None, + cloud_name: str, image_arch: state.Arch, resource_prefix: str ) -> None: """Initialize github-runner-image-builder app. @@ -151,7 +145,6 @@ def _initialize_image_builder( cloud_name: The OpenStack cloud to pre-populate OpenStack image builder resources. image_arch: The architecture of the image to build. resource_prefix: The resource prefix for artefacts saved in the image repository. - proxy_config: The proxy configuration to apply to the environment. Raises: ImageBuilderInitializeError: If there was an error Initialize the app. From 47c6b97b44a26ebc62cb908f583b24608809fa58 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 14:57:19 +0530 Subject: [PATCH 22/38] remove code --- src/builder.py | 2 +- src/charm.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/builder.py b/src/builder.py index 3bf47cde..c2ed78ae 100644 --- a/src/builder.py +++ b/src/builder.py @@ -137,7 +137,7 @@ def _install_app() -> None: def _initialize_image_builder( - cloud_name: str, image_arch: state.Arch, resource_prefix: str + cloud_name: str, image_arch: state.Arch, resource_prefix: str ) -> None: """Initialize github-runner-image-builder app. diff --git a/src/charm.py b/src/charm.py index 53282a75..8bbb0d06 100755 --- a/src/charm.py +++ b/src/charm.py @@ -202,7 +202,6 @@ def _setup_builder(self) -> None: image_arch=builder_config_state.image_config.arch, resource_prefix=builder_config_state.app_config.resource_prefix, unit_name=self.unit.name, - proxy=builder_config_state.proxy, ) ) From b804d34b61586fb2b2428e178f23df011e5c9261 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 15:00:25 +0530 Subject: [PATCH 23/38] setup proxy env for all hooks --- src/charm.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/charm.py b/src/charm.py index 8bbb0d06..cdecae48 100755 --- a/src/charm.py +++ b/src/charm.py @@ -102,6 +102,7 @@ def _on_upgrade_charm(self, _: ops.UpgradeCharmEvent) -> None: def _on_config_changed(self, _: ops.ConfigChangedEvent) -> None: """Handle charm configuration change events.""" builder_config_state = state.BuilderConfig.from_charm(charm=self) + self._setup_proxy_environment(builder_config_state.proxy) if not self._is_any_image_relation_ready(cloud_config=builder_config_state.cloud_config): return # The following lines should be covered by integration tests. @@ -118,6 +119,7 @@ def _on_config_changed(self, _: ops.ConfigChangedEvent) -> None: def _on_image_relation_changed(self, evt: ops.RelationChangedEvent) -> None: """Handle charm image relation changed event.""" builder_config_state = state.BuilderConfig.from_charm(charm=self) + self._setup_proxy_environment(builder_config_state.proxy) if not evt.unit: logger.info("No unit in image relation changed event. Skipping image building.") return @@ -156,6 +158,7 @@ def _on_image_relation_changed(self, evt: ops.RelationChangedEvent) -> None: def _on_run(self, _: RunEvent) -> None: """Handle the run event.""" builder_config_state = state.BuilderConfig.from_charm(charm=self) + self._setup_proxy_environment(builder_config_state.proxy) if not self._is_any_image_relation_ready(cloud_config=builder_config_state.cloud_config): return # The following line should be covered by the integration test. @@ -169,6 +172,7 @@ def _on_run_action(self, event: ops.ActionEvent) -> None: event: The run action event. """ builder_config_state = state.BuilderConfig.from_charm(charm=self) + self._setup_proxy_environment(builder_config_state.proxy) if not self._is_any_image_relation_ready(cloud_config=builder_config_state.cloud_config): event.fail("Image relation not yet ready.") return From b57f7ae797080fdd26becdbc2efae5f256864c21 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 17:34:01 +0530 Subject: [PATCH 24/38] fix lint issues --- tests/unit/test_builder.py | 2 -- tests/unit/test_charm.py | 6 ++---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index f731d691..93456a13 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -67,7 +67,6 @@ def test_initialize(monkeypatch: pytest.MonkeyPatch, tmp_path: Path): image_arch=MagicMock(), resource_prefix=MagicMock(), unit_name=MagicMock(), - proxy=None, ) ) @@ -179,7 +178,6 @@ def test__initialize_image_builder_error( cloud_name=MagicMock(), image_arch=MagicMock(), resource_prefix=MagicMock(), - proxy_config=None, ) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index de96d77a..67117818 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -38,7 +38,7 @@ def mock_builder_fixture(monkeypatch: pytest.MonkeyPatch): monkeypatch.setattr( state.BuilderConfig, "from_charm", - MagicMock(return_value=MagicMock(image_config=image_config)), + MagicMock(return_value=MagicMock(image_config=image_config, proxy=None)), ) monkeypatch.setattr(builder, "install_clouds_yaml", MagicMock()) monkeypatch.setattr(builder, "get_latest_images", MagicMock(return_value=[])) @@ -135,10 +135,8 @@ def test_installation( act: when _on_install is called. assert: setup_builder is called. """ - builder_config_mock = MagicMock() - builder_config_mock.proxy = None monkeypatch.setattr( - state.BuilderConfig, "from_charm", MagicMock(return_value=builder_config_mock) + state.BuilderConfig, "from_charm", MagicMock(return_value=MagicMock(proxy=None)) ) monkeypatch.setattr(image, "Observer", MagicMock()) monkeypatch.setattr(builder, "initialize", (builder_setup_mock := MagicMock())) From 40db4e1a1a9972b2fc07f4d0809600fe801ecdd9 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 17:37:22 +0530 Subject: [PATCH 25/38] remove lint --- tests/unit/test_builder.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 93456a13..1370a474 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -175,9 +175,7 @@ def test__initialize_image_builder_error( with pytest.raises(builder.ImageBuilderInitializeError): builder._initialize_image_builder( - cloud_name=MagicMock(), - image_arch=MagicMock(), - resource_prefix=MagicMock(), + cloud_name=MagicMock(), image_arch=MagicMock(), resource_prefix=MagicMock() ) From 60cd8636d6c917f0961f1af5aa03e4ff19f820cf Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 17:52:11 +0530 Subject: [PATCH 26/38] add more logs --- app/pyproject.toml | 2 +- app/src/github_runner_image_builder/cloud_image.py | 5 +++++ src/builder.py | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/pyproject.toml b/app/pyproject.toml index 386db293..2da3b106 100644 --- a/app/pyproject.toml +++ b/app/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.12.0" +version = "0.12.1" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] diff --git a/app/src/github_runner_image_builder/cloud_image.py b/app/src/github_runner_image_builder/cloud_image.py index b16725c5..9000f77f 100644 --- a/app/src/github_runner_image_builder/cloud_image.py +++ b/app/src/github_runner_image_builder/cloud_image.py @@ -8,6 +8,7 @@ import gzip # noqa: F401 # pylint: disable=unused-import import hashlib import logging +import os import typing from datetime import date from pathlib import Path @@ -121,6 +122,7 @@ def _download_base_image( # The ubuntu-cloud-images is a trusted source # Bandit thinks there is no timeout provided for the code below. try: + logger.info(f"Env inside image builder binary: {os.environ}") request = requests.get( f"https://cloud-images.ubuntu.com/{base_image.value}/{release_dir}/{base_image.value}" f"-server-cloudimg-{bin_arch}.img", @@ -188,3 +190,6 @@ def _validate_checksum(file: Path, expected_checksum: str) -> bool: while data := target_file.read(CHECKSUM_BUF_SIZE): sha256.update(data) return sha256.hexdigest() == expected_checksum + while data := target_file.read(CHECKSUM_BUF_SIZE): + sha256.update(data) + return sha256.hexdigest() == expected_checksum diff --git a/src/builder.py b/src/builder.py index c2ed78ae..e252c491 100644 --- a/src/builder.py +++ b/src/builder.py @@ -189,6 +189,7 @@ def _build_init_command( """ cmd = [ "/usr/bin/sudo", + "--preserve-env", str(GITHUB_RUNNER_IMAGE_BUILDER_PATH), "--os-cloud", cloud_name, From 130b7f75ca0e052119b6f357cb984566c39bc614 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 17:53:53 +0530 Subject: [PATCH 27/38] remove randomly added code --- app/src/github_runner_image_builder/cloud_image.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/src/github_runner_image_builder/cloud_image.py b/app/src/github_runner_image_builder/cloud_image.py index 9000f77f..369de33f 100644 --- a/app/src/github_runner_image_builder/cloud_image.py +++ b/app/src/github_runner_image_builder/cloud_image.py @@ -190,6 +190,3 @@ def _validate_checksum(file: Path, expected_checksum: str) -> bool: while data := target_file.read(CHECKSUM_BUF_SIZE): sha256.update(data) return sha256.hexdigest() == expected_checksum - while data := target_file.read(CHECKSUM_BUF_SIZE): - sha256.update(data) - return sha256.hexdigest() == expected_checksum From cc9f619dfe903c18c1a917e949c66aea6848ea52 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 10 Feb 2026 18:05:15 +0530 Subject: [PATCH 28/38] lint issues --- app/src/github_runner_image_builder/cloud_image.py | 2 +- tests/unit/test_builder.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src/github_runner_image_builder/cloud_image.py b/app/src/github_runner_image_builder/cloud_image.py index 369de33f..7068c531 100644 --- a/app/src/github_runner_image_builder/cloud_image.py +++ b/app/src/github_runner_image_builder/cloud_image.py @@ -122,7 +122,7 @@ def _download_base_image( # The ubuntu-cloud-images is a trusted source # Bandit thinks there is no timeout provided for the code below. try: - logger.info(f"Env inside image builder binary: {os.environ}") + logger.info("Env inside image builder binary: %s", os.environ) request = requests.get( f"https://cloud-images.ubuntu.com/{base_image.value}/{release_dir}/{base_image.value}" f"-server-cloudimg-{bin_arch}.img", diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 1370a474..045625f8 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -186,6 +186,7 @@ def test__initialize_image_builder_error( None, [ "/usr/bin/sudo", + "--preserve-env", "/home/ubuntu/.local/bin/github-runner-image-builder", "--os-cloud", "test-cloud-name", @@ -199,6 +200,7 @@ def test__initialize_image_builder_error( "test-prefix", [ "/usr/bin/sudo", + "--preserve-env", "/home/ubuntu/.local/bin/github-runner-image-builder", "--os-cloud", "test-cloud-name", From d3befa1e6783f060dae32c54220efb020fe90e4f Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Wed, 11 Feb 2026 11:49:03 +0530 Subject: [PATCH 29/38] disable noproxy and check --- tests/integration/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index c2ae0094..1f3c0923 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -105,7 +105,7 @@ async def model_fixture(proxy: ProxyConfig, ops_test: OpsTest) -> AsyncGenerator { "juju-http-proxy": proxy.http, "juju-https-proxy": proxy.https, - "juju-no-proxy": proxy.no_proxy, + #"juju-no-proxy": proxy.no_proxy, "apt-http-proxy": proxy.http, "apt-https-proxy": proxy.https, "snap-http-proxy": proxy.http, From 608545b0e0f72021cc0c22d0d613fbcfcb98fb95 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Wed, 11 Feb 2026 12:39:39 +0530 Subject: [PATCH 30/38] experiment with no proxy --- tests/integration/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 1f3c0923..5174fa95 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -105,7 +105,7 @@ async def model_fixture(proxy: ProxyConfig, ops_test: OpsTest) -> AsyncGenerator { "juju-http-proxy": proxy.http, "juju-https-proxy": proxy.https, - #"juju-no-proxy": proxy.no_proxy, + "juju-no-proxy": "127.0.0.1,localhost,::1,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,.launchpad.net,.internal,.jujucharms.com,.nip.io", "apt-http-proxy": proxy.http, "apt-https-proxy": proxy.https, "snap-http-proxy": proxy.http, From 51761f93bebf1ba5781522afc7579f2049ed47e0 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Thu, 12 Feb 2026 10:02:33 +0530 Subject: [PATCH 31/38] debug --- tests/integration/conftest.py | 5 +---- tests/integration/test_charm.py | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 5174fa95..7283a6e5 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -105,7 +105,7 @@ async def model_fixture(proxy: ProxyConfig, ops_test: OpsTest) -> AsyncGenerator { "juju-http-proxy": proxy.http, "juju-https-proxy": proxy.https, - "juju-no-proxy": "127.0.0.1,localhost,::1,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,.launchpad.net,.internal,.jujucharms.com,.nip.io", + "juju-no-proxy": proxy.no_proxy, "apt-http-proxy": proxy.http, "apt-https-proxy": proxy.https, "snap-http-proxy": proxy.http, @@ -124,9 +124,6 @@ def dispatch_time_fixture(): @pytest.fixture(scope="module", name="test_charm_file") def test_charm_file_fixture() -> str: """Build the charm and return the path to the built charm.""" - subprocess.check_call( # nosec: B603 - ["/snap/bin/charmcraft", "pack", "-p", "tests/integration/data/charm"] - ) return "./test_ubuntu-22.04-amd64.charm" diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 32665248..f3314e44 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -7,6 +7,7 @@ import json import logging +import subprocess from contextlib import asynccontextmanager from datetime import datetime, timezone @@ -18,11 +19,24 @@ from pytest_operator.plugin import OpsTest from builder import CRON_BUILD_SCHEDULE_PATH -from state import BUILD_INTERVAL_CONFIG_NAME +from state import BUILD_INTERVAL_CONFIG_NAME, ProxyConfig from tests.integration.helpers import image_created_from_dispatch, wait_for_images logger = logging.getLogger(__name__) +@pytest.mark.asyncio +async def test_charmcraft_pack(model: Model, proxy: ProxyConfig): + subprocess.check_call( # nosec: B603 + ["/snap/bin/charmcraft", "pack", "-p", "tests/integration/data/charm"] + ) + if proxy.http: + logger.info("Setting model proxy: %s", proxy.http) + await model.set_config( + { + "juju-no-proxy": "127.0.0.1,localhost,::1,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,.launchpad.net,.internal,.jujucharms.com,.nip.io", + } + ) + @pytest.mark.asyncio async def test_image_relation(app: Application, test_charm: Application): From ca7e35ef89ad78072f4586b82565d1cf3ac05b0c Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Thu, 12 Feb 2026 10:47:17 +0530 Subject: [PATCH 32/38] debug --- tests/integration/test_charm.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index f3314e44..d1a811ad 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -29,11 +29,12 @@ async def test_charmcraft_pack(model: Model, proxy: ProxyConfig): subprocess.check_call( # nosec: B603 ["/snap/bin/charmcraft", "pack", "-p", "tests/integration/data/charm"] ) + logging.info("Charmcraft pack completed successfully.") if proxy.http: logger.info("Setting model proxy: %s", proxy.http) await model.set_config( { - "juju-no-proxy": "127.0.0.1,localhost,::1,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,.launchpad.net,.internal,.jujucharms.com,.nip.io", + "juju-no-proxy": "", } ) From 210216da7146b96d23db10beaef711133280b99d Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Thu, 12 Feb 2026 12:56:16 +0530 Subject: [PATCH 33/38] debug --- tests/integration/conftest.py | 2 +- tests/integration/pre_run_script.sh | 1 + tests/integration/test_charm.py | 14 -------------- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 7283a6e5..7d878391 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -105,7 +105,7 @@ async def model_fixture(proxy: ProxyConfig, ops_test: OpsTest) -> AsyncGenerator { "juju-http-proxy": proxy.http, "juju-https-proxy": proxy.https, - "juju-no-proxy": proxy.no_proxy, + "juju-no-proxy": "", "apt-http-proxy": proxy.http, "apt-https-proxy": proxy.https, "snap-http-proxy": proxy.http, diff --git a/tests/integration/pre_run_script.sh b/tests/integration/pre_run_script.sh index d3e8d017..d1eb4740 100644 --- a/tests/integration/pre_run_script.sh +++ b/tests/integration/pre_run_script.sh @@ -5,6 +5,7 @@ set -euo pipefail +/snap/bin/charmcraft pack -p tests/integration/data/charm lxc list IP=$(lxc list -c 4 --format csv | awk '{print $1}' | head -n 1) echo "IP=$IP" diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index d1a811ad..24a95870 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -24,20 +24,6 @@ logger = logging.getLogger(__name__) -@pytest.mark.asyncio -async def test_charmcraft_pack(model: Model, proxy: ProxyConfig): - subprocess.check_call( # nosec: B603 - ["/snap/bin/charmcraft", "pack", "-p", "tests/integration/data/charm"] - ) - logging.info("Charmcraft pack completed successfully.") - if proxy.http: - logger.info("Setting model proxy: %s", proxy.http) - await model.set_config( - { - "juju-no-proxy": "", - } - ) - @pytest.mark.asyncio async def test_image_relation(app: Application, test_charm: Application): From 2cb34b4b45042b83fbc4e6df962b4b8cefaf35f6 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 17 Feb 2026 10:24:33 +0530 Subject: [PATCH 34/38] remove integration test --- .github/workflows/integration_test.yaml | 3 --- app/pyproject.toml | 2 +- app/src/github_runner_image_builder/cloud_image.py | 2 -- src/builder.py | 1 - tests/integration/conftest.py | 9 ++++----- tests/integration/pre_run_script.sh | 12 ------------ tests/integration/test_charm.py | 3 +-- 7 files changed, 6 insertions(+), 26 deletions(-) delete mode 100644 tests/integration/pre_run_script.sh diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index aba5628e..ebd3c341 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -23,9 +23,6 @@ jobs: self-hosted-runner-arch: X64 self-hosted-runner-label: pfe-ci builder-runner-label: X64 - pre-run-script: | - -c "chmod +x tests/integration/pre_run_script.sh - ./tests/integration/pre_run_script.sh" allure-report: if: ${{ !cancelled() && github.event_name == 'schedule' }} needs: diff --git a/app/pyproject.toml b/app/pyproject.toml index 2da3b106..386db293 100644 --- a/app/pyproject.toml +++ b/app/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.12.1" +version = "0.12.0" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] diff --git a/app/src/github_runner_image_builder/cloud_image.py b/app/src/github_runner_image_builder/cloud_image.py index 7068c531..b16725c5 100644 --- a/app/src/github_runner_image_builder/cloud_image.py +++ b/app/src/github_runner_image_builder/cloud_image.py @@ -8,7 +8,6 @@ import gzip # noqa: F401 # pylint: disable=unused-import import hashlib import logging -import os import typing from datetime import date from pathlib import Path @@ -122,7 +121,6 @@ def _download_base_image( # The ubuntu-cloud-images is a trusted source # Bandit thinks there is no timeout provided for the code below. try: - logger.info("Env inside image builder binary: %s", os.environ) request = requests.get( f"https://cloud-images.ubuntu.com/{base_image.value}/{release_dir}/{base_image.value}" f"-server-cloudimg-{bin_arch}.img", diff --git a/src/builder.py b/src/builder.py index e252c491..5bb0462d 100644 --- a/src/builder.py +++ b/src/builder.py @@ -152,7 +152,6 @@ def _initialize_image_builder( init_cmd = _build_init_command( cloud_name=cloud_name, image_arch=image_arch, resource_prefix=resource_prefix ) - logger.info("Initialize image builder with env: %s", os.environ) try: subprocess.run( init_cmd, diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 7d878391..96bc13c9 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -105,11 +105,7 @@ async def model_fixture(proxy: ProxyConfig, ops_test: OpsTest) -> AsyncGenerator { "juju-http-proxy": proxy.http, "juju-https-proxy": proxy.https, - "juju-no-proxy": "", - "apt-http-proxy": proxy.http, - "apt-https-proxy": proxy.https, - "snap-http-proxy": proxy.http, - "snap-https-proxy": proxy.https, + "juju-no-proxy": proxy.no_proxy, } ) yield ops_test.model @@ -124,6 +120,9 @@ def dispatch_time_fixture(): @pytest.fixture(scope="module", name="test_charm_file") def test_charm_file_fixture() -> str: """Build the charm and return the path to the built charm.""" + subprocess.check_call( # nosec: B603 + ["/snap/bin/charmcraft", "pack", "-p", "tests/integration/data/charm"] + ) return "./test_ubuntu-22.04-amd64.charm" diff --git a/tests/integration/pre_run_script.sh b/tests/integration/pre_run_script.sh deleted file mode 100644 index d1eb4740..00000000 --- a/tests/integration/pre_run_script.sh +++ /dev/null @@ -1,12 +0,0 @@ -#!/bin/bash - -# Copyright 2026 Canonical Ltd. -# See LICENSE file for licensing details. - -set -euo pipefail - -/snap/bin/charmcraft pack -p tests/integration/data/charm -lxc list -IP=$(lxc list -c 4 --format csv | awk '{print $1}' | head -n 1) -echo "IP=$IP" -sudo nft insert rule ip aproxy prerouting index 0 ip saddr != "$IP" return diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 24a95870..32665248 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -7,7 +7,6 @@ import json import logging -import subprocess from contextlib import asynccontextmanager from datetime import datetime, timezone @@ -19,7 +18,7 @@ from pytest_operator.plugin import OpsTest from builder import CRON_BUILD_SCHEDULE_PATH -from state import BUILD_INTERVAL_CONFIG_NAME, ProxyConfig +from state import BUILD_INTERVAL_CONFIG_NAME from tests.integration.helpers import image_created_from_dispatch, wait_for_images logger = logging.getLogger(__name__) From 5c7ec25aca0633020c41313ebfb068c7d39df24b Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 13 Feb 2026 06:54:41 +0000 Subject: [PATCH 35/38] chore(deps): update all non-major dependencies (#190) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/integration_test_app.yaml | 2 +- app/requirements.txt | 2 +- requirements.txt | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/integration_test_app.yaml b/.github/workflows/integration_test_app.yaml index 2c58e729..089ee6a5 100644 --- a/.github/workflows/integration_test_app.yaml +++ b/.github/workflows/integration_test_app.yaml @@ -28,7 +28,7 @@ jobs: - image: jammy arch: s390x steps: - - uses: actions/checkout@v6.0.1 + - uses: actions/checkout@v6.0.2 - uses: canonical/setup-lxd@v0.1.3 - uses: actions/setup-python@v6 with: diff --git a/app/requirements.txt b/app/requirements.txt index 154d506c..6166af77 100644 --- a/app/requirements.txt +++ b/app/requirements.txt @@ -3,5 +3,5 @@ fabric==3.2.2 Jinja2==3.1.6 openstacksdk==4.9.0 pyyaml==6.0.3 -tenacity==9.1.2 +tenacity==9.1.4 typing_extensions==4.15.0 diff --git a/requirements.txt b/requirements.txt index 29518399..5a334938 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ -ops ==3.5.0 +ops ==3.5.2 openstacksdk ==4.9.0 pydantic ==2.12.5 -tenacity ==9.1.2 +tenacity ==9.1.4 cosl ==1.4.0 From 960d6b39167e335451c56a921c99d8447566734a Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 17 Feb 2026 10:27:18 +0530 Subject: [PATCH 36/38] feat: otel-collector snap (#195) * feat: otel-collector snap * chore: application bump related changes * fix: call installation func --- app/pyproject.toml | 4 ++-- .../github_runner_image_builder/templates/cloud-init.sh.j2 | 7 +++++++ app/tests/integration/commands.py | 7 +++++++ app/tests/unit/test_openstack_builder.py | 7 +++++++ docs/changelog.md | 3 +++ 5 files changed, 26 insertions(+), 2 deletions(-) diff --git a/app/pyproject.toml b/app/pyproject.toml index 386db293..1deff3ce 100644 --- a/app/pyproject.toml +++ b/app/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-image-builder" -version = "0.12.0" +version = "0.13.0" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] @@ -32,7 +32,7 @@ dependencies = { file = ["requirements.txt"] } [tool.pydocstyle] match = "src/github_runner_image_builder/cli.py" -ignore = ["D301"] # ignore backshlashes in docstring - click lib uses them. +ignore = ["D301"] # ignore backshlashes in docstring - click lib uses them. [tool.bandit] diff --git a/app/src/github_runner_image_builder/templates/cloud-init.sh.j2 b/app/src/github_runner_image_builder/templates/cloud-init.sh.j2 index 1f905ee9..f13fe4b4 100644 --- a/app/src/github_runner_image_builder/templates/cloud-init.sh.j2 +++ b/app/src/github_runner_image_builder/templates/cloud-init.sh.j2 @@ -120,6 +120,12 @@ function install_yq() { /usr/bin/snap remove go } +function install_opentelemetry_collector_snap() { + echo "Installing OpenTelemetry Collector snap" + /usr/bin/sudo snap install opentelemetry-collector --stable + /usr/bin/sudo snap disable opentelemetry-collector +} + function install_github_runner() { version="$1" arch="$2" @@ -172,6 +178,7 @@ if [ $RELEASE != "focal" ]; then install_yarn fi install_yq +install_opentelemetry_collector_snap install_github_runner "$github_runner_version" "$github_runner_arch" chown_home configure_system_users diff --git a/app/tests/integration/commands.py b/app/tests/integration/commands.py index e7d3e740..112f9b82 100644 --- a/app/tests/integration/commands.py +++ b/app/tests/integration/commands.py @@ -38,6 +38,13 @@ class Commands: name="check aproxy", command="sudo snap info aproxy && sudo snap services aproxy", ), + Commands( + name="check opentelemetry-collector snap installed and inactive", + command=( + "sudo snap services opentelemetry-collector | grep -q 'disabled' " + "&& sudo snap services opentelemetry-collector | grep -q 'inactive'" + ), + ), Commands(name="update apt in docker", command="docker run python:3.10-slim apt-get update"), Commands(name="docker version", command="docker version"), Commands(name="check python3 alias", command="python --version"), diff --git a/app/tests/unit/test_openstack_builder.py b/app/tests/unit/test_openstack_builder.py index 33099b40..2a1278e2 100644 --- a/app/tests/unit/test_openstack_builder.py +++ b/app/tests/unit/test_openstack_builder.py @@ -831,6 +831,12 @@ def test__generate_cloud_init_script( /usr/bin/snap remove go }} +function install_opentelemetry_collector_snap() {{ + echo "Installing OpenTelemetry Collector snap" + /usr/bin/sudo snap install opentelemetry-collector --stable + /usr/bin/sudo snap disable opentelemetry-collector +}} + function install_github_runner() {{ version="$1" arch="$2" @@ -884,6 +890,7 @@ def test__generate_cloud_init_script( install_yarn fi install_yq +install_opentelemetry_collector_snap install_github_runner "$github_runner_version" "$github_runner_arch" chown_home configure_system_users\ diff --git a/docs/changelog.md b/docs/changelog.md index 6ebb21fc..1c87110d 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -4,6 +4,9 @@ ## [#185 Remove aproxy installation and add proxy support in workload](https://github.com/canonical/github-runner-image-builder-operator/pull/185) (2026-01-20) * Remove `aproxy` snap installation in the charm and inject proxy values from the model config into the workload process. +## [#195 feat: otel-collector snap](https://github.com/canonical/github-runner-image-builder-operator/pull/195) (2026-02-11) +* Install opentelemetry collector snap in the runner image. + ## [#172 feat: apt upgrade](https://github.com/canonical/github-runner-image-builder-operator/pull/172) (2025-11-26) * Apply apt-update and apt-upgrade to GH runner images by applying them during cloud-init. From 51782dcac4c27a15ae76275186a9c7aba4db2ceb Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 17 Feb 2026 12:44:24 +0530 Subject: [PATCH 37/38] address review comments --- src/builder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/builder.py b/src/builder.py index 5bb0462d..8aa51d91 100644 --- a/src/builder.py +++ b/src/builder.py @@ -188,7 +188,7 @@ def _build_init_command( """ cmd = [ "/usr/bin/sudo", - "--preserve-env", + "--preserve-env=http_proxy,https_proxy,no_proxy,HTTP_PROXY,HTTPS_PROXY,NO_PROXY", str(GITHUB_RUNNER_IMAGE_BUILDER_PATH), "--os-cloud", cloud_name, @@ -529,7 +529,7 @@ def _run(config: RunConfig) -> list[CloudImage]: env={ "HOME": str(UBUNTU_HOME), **_transform_secrets(secrets=config.image.script_config.script_secrets), - **os.environ, + **{k: v for (k, v) in os.environ.items() if "proxy" in k.lower()}, }, ) # The return value of the CLI is "Image build success:\n" From cac7063f2055ce1f5fc278203363c16d540546b9 Mon Sep 17 00:00:00 2001 From: swetha1654 Date: Tue, 17 Feb 2026 12:47:16 +0530 Subject: [PATCH 38/38] fix unit test --- tests/unit/test_builder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_builder.py b/tests/unit/test_builder.py index 045625f8..57e16e15 100644 --- a/tests/unit/test_builder.py +++ b/tests/unit/test_builder.py @@ -186,7 +186,7 @@ def test__initialize_image_builder_error( None, [ "/usr/bin/sudo", - "--preserve-env", + "--preserve-env=http_proxy,https_proxy,no_proxy,HTTP_PROXY,HTTPS_PROXY,NO_PROXY", "/home/ubuntu/.local/bin/github-runner-image-builder", "--os-cloud", "test-cloud-name", @@ -200,7 +200,7 @@ def test__initialize_image_builder_error( "test-prefix", [ "/usr/bin/sudo", - "--preserve-env", + "--preserve-env=http_proxy,https_proxy,no_proxy,HTTP_PROXY,HTTPS_PROXY,NO_PROXY", "/home/ubuntu/.local/bin/github-runner-image-builder", "--os-cloud", "test-cloud-name",