From 0a04104e9f3e7d04fa83f221b53f29171c4c6116 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 17 Apr 2026 12:33:58 +0800 Subject: [PATCH 01/13] feat: use juju secrets --- charmcraft.yaml | 10 +++---- src/state.py | 22 ++++++++++++++-- tests/integration/conftest.py | 27 ++++++++++++++++--- tests/unit/conftest.py | 7 ++++- tests/unit/factories.py | 13 +++++++++- tests/unit/test_state.py | 49 +++++++++++++++++++++++++++++++++++ 6 files changed, 116 insertions(+), 12 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index 56824386..bc211403 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -108,12 +108,12 @@ config: client (e.g. http://my-openstack-deployment/openstack-keystone). See https://docs.\ openstack.org/python-openstackclient/queens/configuration/index.html for more information. openstack-password: - type: string - default: "" + type: secret description: | - The password section of the clouds.yaml contents, used to authenticate the OpenStack \ - client (e.g. myverysecurepassword). See https://docs.openstack.org/python-openstackclient/\ - queens/configuration/index.html for more information. + The password section of the clouds.yaml contents, used to authenticate the OpenStack + client. A Juju user secret ID should be passed in the format of secret:. + The secret must contain a 'password' key with the OpenStack password as its value. + Example: juju add-secret openstack-password password=. openstack-project-domain-name: type: string default: "" diff --git a/src/state.py b/src/state.py index 506d68f2..41de10ad 100644 --- a/src/state.py +++ b/src/state.py @@ -628,14 +628,32 @@ def _parse_openstack_clouds_config(charm: ops.CharmBase) -> OpenstackCloudsConfi The openstack clouds yaml. """ auth_url = typing.cast(str, charm.config.get(OPENSTACK_AUTH_URL_CONFIG_NAME)) - password = typing.cast(str, charm.config.get(OPENSTACK_PASSWORD_CONFIG_NAME)) + password_secret_id = typing.cast(str, charm.config.get(OPENSTACK_PASSWORD_CONFIG_NAME)) project_domain = typing.cast(str, charm.config.get(OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME)) project = typing.cast(str, charm.config.get(OPENSTACK_PROJECT_CONFIG_NAME)) user_domain = typing.cast(str, charm.config.get(OPENSTACK_USER_DOMAIN_CONFIG_NAME)) user = typing.cast(str, charm.config.get(OPENSTACK_USER_CONFIG_NAME)) - if not all((auth_url, password, project_domain, project, user_domain, user)): + if not all((auth_url, password_secret_id, project_domain, project, user_domain, user)): raise InvalidCloudConfigError("Please supply all OpenStack configurations.") + try: + secret = charm.model.get_secret(id=password_secret_id) + except ops.SecretNotFoundError as exc: + raise InvalidCloudConfigError( + f"OpenStack password secret not found: {password_secret_id}." + ) from exc + except ops.ModelError as exc: + raise InvalidCloudConfigError( + "Charm does not have access to the OpenStack password secret. " + "Please grant the charm read access to the secret." + ) from exc + secret_content = secret.get_content(refresh=True) + password = secret_content.get("password", "") + if not password: + raise InvalidCloudConfigError( + f"Secret {password_secret_id} does not contain a 'password' key." + ) + clouds_config = OpenstackCloudsConfig( clouds={ CLOUD_NAME: _CloudsConfig( diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 1d130d93..aea69a7a 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -332,12 +332,27 @@ async def script_secret_fixture(test_configs) -> _Secret: return _Secret(id=secret_id, name=secret_name) -@pytest.fixture(scope="module", name="app_config") -def app_config_fixture( +@pytest_asyncio.fixture(scope="module", name="openstack_password_secret") +async def openstack_password_secret_fixture( + test_configs: TestConfigs, + private_endpoint_configs: PrivateEndpointConfigs, +) -> _Secret: + """The OpenStack password Juju secret.""" + secret_name = f"openstack-password-{uuid4().hex}" + secret_id = await test_configs.model.add_secret( + name=secret_name, + data_args=[f"password={private_endpoint_configs['password']}"], + ) # note secret_id already contains "secret:" prefix + return _Secret(id=secret_id, name=secret_name) + + +@pytest_asyncio.fixture(scope="module", name="app_config") +async def app_config_fixture( private_endpoint_configs: PrivateEndpointConfigs, image_configs: ImageConfigs, openstack_metadata: OpenstackMeta, arch: state.Arch, + openstack_password_secret: _Secret, ) -> dict: """The image builder application config.""" return { @@ -346,7 +361,7 @@ def app_config_fixture( BUILD_INTERVAL_CONFIG_NAME: 12, REVISION_HISTORY_LIMIT_CONFIG_NAME: 5, OPENSTACK_AUTH_URL_CONFIG_NAME: private_endpoint_configs["auth_url"], - OPENSTACK_PASSWORD_CONFIG_NAME: private_endpoint_configs["password"], + OPENSTACK_PASSWORD_CONFIG_NAME: openstack_password_secret.id, OPENSTACK_PROJECT_CONFIG_NAME: private_endpoint_configs["project_name"], OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME: private_endpoint_configs["project_domain_name"], OPENSTACK_USER_CONFIG_NAME: private_endpoint_configs["username"], @@ -372,6 +387,7 @@ async def app_fixture( base_machine_constraint: str, test_configs: TestConfigs, script_secret: _Secret, + openstack_password_secret: _Secret, ) -> AsyncGenerator[Application, None]: """The deployed application fixture.""" logger.info("Deploying image builder: %s", test_configs.dispatch_time) @@ -381,6 +397,7 @@ async def app_fixture( constraints=base_machine_constraint, config=app_config, ) + await app.model.grant_secret(openstack_password_secret.name, app.name) await app.model.grant_secret(script_secret.name, app.name) await app.set_config( { @@ -404,6 +421,7 @@ async def app_on_charmhub_fixture( app_config: dict, base_machine_constraint: str, ops_test, + openstack_password_secret: _Secret, ) -> AsyncGenerator[Application, None]: """Fixture for deploying the charm from charmhub.""" # Normally we would use latest/stable, but upgrading @@ -430,6 +448,9 @@ async def app_on_charmhub_fixture( channel=charmhub_channel, ) + if OPENSTACK_PASSWORD_CONFIG_NAME in charmhub_config_options: + await app.model.grant_secret(openstack_password_secret.name, app.name) + await test_configs.model.wait_for_idle(apps=[app.name], idle_period=30, timeout=60 * 30) yield app diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 7f90fe30..61473042 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -25,13 +25,18 @@ def harness_fixture(): harness = Harness(GithubRunnerImageBuilderCharm) harness.begin() + openstack_password_secret_id = harness.add_user_secret( + {"password": secrets.token_hex(16)} + ) + harness.grant_secret(openstack_password_secret_id, harness.charm.app.name) + # Replace config_changed handler temporarily. config_changed_handler = harness.charm._on_config_changed harness.charm._on_config_changed = MagicMock() harness.update_config( { state.OPENSTACK_AUTH_URL_CONFIG_NAME: "https://test-auth-url.com/", - state.OPENSTACK_PASSWORD_CONFIG_NAME: secrets.token_hex(16), + state.OPENSTACK_PASSWORD_CONFIG_NAME: openstack_password_secret_id, state.OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME: "test", state.OPENSTACK_PROJECT_CONFIG_NAME: "test", state.OPENSTACK_USER_DOMAIN_CONFIG_NAME: "test", diff --git a/tests/unit/factories.py b/tests/unit/factories.py index 153480a0..28afd941 100644 --- a/tests/unit/factories.py +++ b/tests/unit/factories.py @@ -76,6 +76,7 @@ class Meta: # pylint: disable=too-few-public-methods """Configuration for factory.""" # noqa: DCO060 model = MagicMock + exclude = ["_setup_mock_openstack_secret"] app = MockAppFactory() unit = MockUnitFactory() @@ -86,7 +87,7 @@ class Meta: # pylint: disable=too-few-public-methods EXTERNAL_BUILD_FLAVOR_CONFIG_NAME: "test-flavor", EXTERNAL_BUILD_NETWORK_CONFIG_NAME: "test-network", OPENSTACK_AUTH_URL_CONFIG_NAME: "http://testing-auth/keystone", - OPENSTACK_PASSWORD_CONFIG_NAME: "test-password", + OPENSTACK_PASSWORD_CONFIG_NAME: "secret:test-secret-id", # nosec: hardcoded_password_string OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME: "test-project-domain", OPENSTACK_PROJECT_CONFIG_NAME: "test-project-name", OPENSTACK_USER_DOMAIN_CONFIG_NAME: "test-user-domain", @@ -98,6 +99,16 @@ class Meta: # pylint: disable=too-few-public-methods } ) + @factory.post_generation + def _setup_mock_openstack_secret( # noqa: DCO020 + obj: MagicMock, create: bool, extracted: typing.Any, **kwargs: typing.Any + ) -> None: + mock_secret = MagicMock() + mock_secret.get_content.return_value = { + "password": "test-password" # nosec: hardcoded_password_string + } + obj.model.get_secret.return_value = mock_secret + class CloudAuthFactory(factory.DictFactory): """Mock cloud auth dict object factory.""" # noqa: DCO060 diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index e451ace4..5352f469 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -422,6 +422,55 @@ def test__parse_openstack_clouds_config(): ) +@pytest.mark.parametrize( + "exception, expected_error", + [ + pytest.param( + ops.SecretNotFoundError, + "OpenStack password secret not found:", + id="secret-not-found", + ), + pytest.param( + ops.ModelError, + "Please grant the charm read access to the secret.", + id="access-denied", + ), + ], +) +def test__parse_openstack_clouds_config_secret_error( + exception: Exception, expected_error: str +): + """ + arrange: given a mocked model get_secret method that raises a given error. + act: when _parse_openstack_clouds_config is called. + assert: InvalidCloudConfigError is raised. + """ + charm = factories.MockCharmFactory() + charm.model.get_secret = MagicMock(side_effect=exception) + + with pytest.raises(state.InvalidCloudConfigError) as exc: + state._parse_openstack_clouds_config(charm) + + assert expected_error in str(exc) + + +def test__parse_openstack_clouds_config_missing_password_key(): + """ + arrange: given a secret that does not contain a 'password' key. + act: when _parse_openstack_clouds_config is called. + assert: InvalidCloudConfigError is raised. + """ + charm = factories.MockCharmFactory() + mock_secret = MagicMock() + mock_secret.get_content.return_value = {} + charm.model.get_secret.return_value = mock_secret + + with pytest.raises(state.InvalidCloudConfigError) as exc: + state._parse_openstack_clouds_config(charm) + + assert "does not contain a 'password' key" in str(exc) + + # pylint: enable=undefined-variable,unused-variable From c0c4c5c709cf81feb1487aef6dacbf1735991e5b Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 17 Apr 2026 04:53:45 +0000 Subject: [PATCH 02/13] test: lint --- tests/integration/conftest.py | 40 +++++++++++++++++++++++++---------- tests/unit/conftest.py | 4 +--- tests/unit/test_state.py | 4 +--- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index aea69a7a..8dbf6475 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -415,17 +415,17 @@ async def app_fixture( await test_configs.model.remove_application(app_name=app.name) -@pytest_asyncio.fixture(scope="module", name="app_on_charmhub") -async def app_on_charmhub_fixture( - test_configs: TestConfigs, - app_config: dict, - base_machine_constraint: str, - ops_test, - openstack_password_secret: _Secret, -) -> AsyncGenerator[Application, None]: - """Fixture for deploying the charm from charmhub.""" - # Normally we would use latest/stable, but upgrading - # from stable is currently broken, and therefore we are using edge. Change this in the future. +async def _prepare_charmhub_app_config(ops_test, app_config: dict) -> tuple[str, dict, set]: + """Prepare the application config for charmhub deployment. + + Args: + ops_test: The pytest operator test instance. + app_config: The base application configuration. + + Returns: + A tuple of (channel, prepared_config, config_options). + + """ charmhub_channel = "edge" ret_code, stdout, stderr = await ops_test.juju( "info", "--format", "json", "--channel", charmhub_channel, "github-runner-image-builder" @@ -440,6 +440,24 @@ async def app_on_charmhub_fixture( for opt in (EXTERNAL_BUILD_FLAVOR_CONFIG_NAME, EXTERNAL_BUILD_NETWORK_CONFIG_NAME): if (legacy_opt := f"{legacy_config_prefix}{opt}") in charmhub_config_options: charmhub_app_config[legacy_opt] = app_config[opt] + + return charmhub_channel, charmhub_app_config, charmhub_config_options + + +@pytest_asyncio.fixture(scope="module", name="app_on_charmhub") +async def app_on_charmhub_fixture( + test_configs: TestConfigs, + app_config: dict, + base_machine_constraint: str, + ops_test, + openstack_password_secret: _Secret, +) -> AsyncGenerator[Application, None]: + """Fixture for deploying the charm from charmhub.""" + # Normally we would use latest/stable, but upgrading + # from stable is currently broken, and therefore we are using edge. Change this in the future. + charmhub_channel, charmhub_app_config, charmhub_config_options = ( + await _prepare_charmhub_app_config(ops_test, app_config) + ) app: Application = await test_configs.model.deploy( "github-runner-image-builder", application_name=f"image-builder-operator-{test_configs.test_id}", diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 61473042..fa3690d5 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -25,9 +25,7 @@ def harness_fixture(): harness = Harness(GithubRunnerImageBuilderCharm) harness.begin() - openstack_password_secret_id = harness.add_user_secret( - {"password": secrets.token_hex(16)} - ) + openstack_password_secret_id = harness.add_user_secret({"password": secrets.token_hex(16)}) harness.grant_secret(openstack_password_secret_id, harness.charm.app.name) # Replace config_changed handler temporarily. diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index 5352f469..9756a4db 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -437,9 +437,7 @@ def test__parse_openstack_clouds_config(): ), ], ) -def test__parse_openstack_clouds_config_secret_error( - exception: Exception, expected_error: str -): +def test__parse_openstack_clouds_config_secret_error(exception: Exception, expected_error: str): """ arrange: given a mocked model get_secret method that raises a given error. act: when _parse_openstack_clouds_config is called. From ce037f05c610634ef921d6467b14f4ca69a72c53 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 17 Apr 2026 04:59:33 +0000 Subject: [PATCH 03/13] docs: changelog --- docs/changelog.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog.md b/docs/changelog.md index d7356b48..5b8ace79 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,6 +1,9 @@ +## [#219 Use Juju secrets] +* Use Juju secrets for sensitive configuration values. + ## [#206 Add resolute image support] * Add resolute image support. From 7faffeb8e978ebddd4a9eff8eb3d49e9185bde96 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 17 Apr 2026 05:49:39 +0000 Subject: [PATCH 04/13] chore: make config backwards compatible --- charmcraft.yaml | 9 +++++++ docs/changelog.md | 4 +++- src/state.py | 45 ++++++++++++++++++++++------------- tests/integration/conftest.py | 6 ++--- tests/unit/conftest.py | 5 +--- tests/unit/factories.py | 4 +++- tests/unit/test_state.py | 32 ++++++++++++++++++++++++- 7 files changed, 78 insertions(+), 27 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index bc211403..ebe42a76 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -108,12 +108,21 @@ config: client (e.g. http://my-openstack-deployment/openstack-keystone). See https://docs.\ openstack.org/python-openstackclient/queens/configuration/index.html for more information. openstack-password: + type: string + default: "" + description: | + The password section of the clouds.yaml contents, used to authenticate the OpenStack \ + client (e.g. myverysecurepassword). See https://docs.openstack.org/python-openstackclient/\ + queens/configuration/index.html for more information. \ + DEPRECATED: Use openstack-password-secret instead for better security. + openstack-password-secret: type: secret description: | The password section of the clouds.yaml contents, used to authenticate the OpenStack client. A Juju user secret ID should be passed in the format of secret:. The secret must contain a 'password' key with the OpenStack password as its value. Example: juju add-secret openstack-password password=. + This option takes precedence over openstack-password if both are set. openstack-project-domain-name: type: string default: "" diff --git a/docs/changelog.md b/docs/changelog.md index 5b8ace79..29147ceb 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,7 +2,9 @@ ## [#219 Use Juju secrets] -* Use Juju secrets for sensitive configuration values. +* Add new `openstack-password-secret` configuration option to securely store OpenStack passwords using Juju secrets. +* Deprecated `openstack-password` configuration option (still supported for backward compatibility). +* Users can migrate to the new secret-based approach by setting `openstack-password-secret` instead of `openstack-password`. ## [#206 Add resolute image support] * Add resolute image support. diff --git a/src/state.py b/src/state.py index 41de10ad..bc4d4773 100644 --- a/src/state.py +++ b/src/state.py @@ -36,6 +36,8 @@ OPENSTACK_AUTH_URL_CONFIG_NAME = "openstack-auth-url" # Bandit thinks this is a hardcoded password OPENSTACK_PASSWORD_CONFIG_NAME = "openstack-password" # nosec: hardcoded_password_string +# nosec: hardcoded_password_string +OPENSTACK_PASSWORD_SECRET_CONFIG_NAME = "openstack-password-secret" 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" @@ -628,30 +630,39 @@ def _parse_openstack_clouds_config(charm: ops.CharmBase) -> OpenstackCloudsConfi The openstack clouds yaml. """ auth_url = typing.cast(str, charm.config.get(OPENSTACK_AUTH_URL_CONFIG_NAME)) - password_secret_id = typing.cast(str, charm.config.get(OPENSTACK_PASSWORD_CONFIG_NAME)) + password_secret_id = typing.cast(str, charm.config.get(OPENSTACK_PASSWORD_SECRET_CONFIG_NAME)) + password = typing.cast(str, charm.config.get(OPENSTACK_PASSWORD_CONFIG_NAME)) project_domain = typing.cast(str, charm.config.get(OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME)) project = typing.cast(str, charm.config.get(OPENSTACK_PROJECT_CONFIG_NAME)) user_domain = typing.cast(str, charm.config.get(OPENSTACK_USER_DOMAIN_CONFIG_NAME)) user = typing.cast(str, charm.config.get(OPENSTACK_USER_CONFIG_NAME)) - if not all((auth_url, password_secret_id, project_domain, project, user_domain, user)): + + # Check if we have the required configs, password can come from either source + if not all((auth_url, project_domain, project, user_domain, user)): raise InvalidCloudConfigError("Please supply all OpenStack configurations.") - try: - secret = charm.model.get_secret(id=password_secret_id) - except ops.SecretNotFoundError as exc: - raise InvalidCloudConfigError( - f"OpenStack password secret not found: {password_secret_id}." - ) from exc - except ops.ModelError as exc: - raise InvalidCloudConfigError( - "Charm does not have access to the OpenStack password secret. " - "Please grant the charm read access to the secret." - ) from exc - secret_content = secret.get_content(refresh=True) - password = secret_content.get("password", "") - if not password: + # Prefer the secret-based password if provided + if password_secret_id: + try: + secret = charm.model.get_secret(id=password_secret_id) + except ops.SecretNotFoundError as exc: + raise InvalidCloudConfigError( + f"OpenStack password secret not found: {password_secret_id}." + ) from exc + except ops.ModelError as exc: + raise InvalidCloudConfigError( + "Charm does not have access to the OpenStack password secret. " + "Please grant the charm read access to the secret." + ) from exc + secret_content = secret.get_content(refresh=True) + password = secret_content.get("password", "") + if not password: + raise InvalidCloudConfigError( + f"Secret {password_secret_id} does not contain a 'password' key." + ) + elif not password: raise InvalidCloudConfigError( - f"Secret {password_secret_id} does not contain a 'password' key." + "Please supply OpenStack password via openstack-password or openstack-password-secret." ) clouds_config = OpenstackCloudsConfig( diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 8dbf6475..81a35cda 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -37,7 +37,7 @@ EXTERNAL_BUILD_FLAVOR_CONFIG_NAME, EXTERNAL_BUILD_NETWORK_CONFIG_NAME, OPENSTACK_AUTH_URL_CONFIG_NAME, - OPENSTACK_PASSWORD_CONFIG_NAME, + OPENSTACK_PASSWORD_SECRET_CONFIG_NAME, OPENSTACK_PROJECT_CONFIG_NAME, OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME, OPENSTACK_USER_CONFIG_NAME, @@ -361,7 +361,7 @@ async def app_config_fixture( BUILD_INTERVAL_CONFIG_NAME: 12, REVISION_HISTORY_LIMIT_CONFIG_NAME: 5, OPENSTACK_AUTH_URL_CONFIG_NAME: private_endpoint_configs["auth_url"], - OPENSTACK_PASSWORD_CONFIG_NAME: openstack_password_secret.id, + OPENSTACK_PASSWORD_SECRET_CONFIG_NAME: openstack_password_secret.id, OPENSTACK_PROJECT_CONFIG_NAME: private_endpoint_configs["project_name"], OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME: private_endpoint_configs["project_domain_name"], OPENSTACK_USER_CONFIG_NAME: private_endpoint_configs["username"], @@ -466,7 +466,7 @@ async def app_on_charmhub_fixture( channel=charmhub_channel, ) - if OPENSTACK_PASSWORD_CONFIG_NAME in charmhub_config_options: + if OPENSTACK_PASSWORD_SECRET_CONFIG_NAME in charmhub_config_options: await app.model.grant_secret(openstack_password_secret.name, app.name) await test_configs.model.wait_for_idle(apps=[app.name], idle_period=30, timeout=60 * 30) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index fa3690d5..7f90fe30 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -25,16 +25,13 @@ def harness_fixture(): harness = Harness(GithubRunnerImageBuilderCharm) harness.begin() - openstack_password_secret_id = harness.add_user_secret({"password": secrets.token_hex(16)}) - harness.grant_secret(openstack_password_secret_id, harness.charm.app.name) - # Replace config_changed handler temporarily. config_changed_handler = harness.charm._on_config_changed harness.charm._on_config_changed = MagicMock() harness.update_config( { state.OPENSTACK_AUTH_URL_CONFIG_NAME: "https://test-auth-url.com/", - state.OPENSTACK_PASSWORD_CONFIG_NAME: openstack_password_secret_id, + state.OPENSTACK_PASSWORD_CONFIG_NAME: secrets.token_hex(16), state.OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME: "test", state.OPENSTACK_PROJECT_CONFIG_NAME: "test", state.OPENSTACK_USER_DOMAIN_CONFIG_NAME: "test", diff --git a/tests/unit/factories.py b/tests/unit/factories.py index 28afd941..d3dfb08f 100644 --- a/tests/unit/factories.py +++ b/tests/unit/factories.py @@ -18,6 +18,7 @@ EXTERNAL_BUILD_NETWORK_CONFIG_NAME, OPENSTACK_AUTH_URL_CONFIG_NAME, OPENSTACK_PASSWORD_CONFIG_NAME, + OPENSTACK_PASSWORD_SECRET_CONFIG_NAME, OPENSTACK_PROJECT_CONFIG_NAME, OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME, OPENSTACK_USER_CONFIG_NAME, @@ -87,7 +88,8 @@ class Meta: # pylint: disable=too-few-public-methods EXTERNAL_BUILD_FLAVOR_CONFIG_NAME: "test-flavor", EXTERNAL_BUILD_NETWORK_CONFIG_NAME: "test-network", OPENSTACK_AUTH_URL_CONFIG_NAME: "http://testing-auth/keystone", - OPENSTACK_PASSWORD_CONFIG_NAME: "secret:test-secret-id", # nosec: hardcoded_password_string + OPENSTACK_PASSWORD_CONFIG_NAME: "", + OPENSTACK_PASSWORD_SECRET_CONFIG_NAME: "secret:test-secret-id", # nosec: hardcoded_password_string OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME: "test-project-domain", OPENSTACK_PROJECT_CONFIG_NAME: "test-project-name", OPENSTACK_USER_DOMAIN_CONFIG_NAME: "test-user-domain", diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index 9756a4db..1143ea39 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -374,7 +374,6 @@ def test__parse_runner_version(version: str, expected_version: str): "missing_config", [ pytest.param(state.OPENSTACK_AUTH_URL_CONFIG_NAME), - pytest.param(state.OPENSTACK_PASSWORD_CONFIG_NAME), pytest.param(state.OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME), pytest.param(state.OPENSTACK_PROJECT_CONFIG_NAME), pytest.param(state.OPENSTACK_USER_DOMAIN_CONFIG_NAME), @@ -469,6 +468,37 @@ def test__parse_openstack_clouds_config_missing_password_key(): assert "does not contain a 'password' key" in str(exc) +def test__parse_openstack_clouds_config_legacy_password(): + """ + arrange: given a charm with the legacy openstack-password config (string). + act: when _parse_openstack_clouds_config is called. + assert: the clouds config is parsed correctly using the legacy password. + """ + charm = factories.MockCharmFactory() + charm.config[state.OPENSTACK_PASSWORD_CONFIG_NAME] = "legacy-password" + charm.config[state.OPENSTACK_PASSWORD_SECRET_CONFIG_NAME] = "" + + clouds_config = state._parse_openstack_clouds_config(charm) + + assert clouds_config.clouds[state.CLOUD_NAME].auth.password == "legacy-password" + + +def test__parse_openstack_clouds_config_no_password(): + """ + arrange: given a charm with neither password config set. + act: when _parse_openstack_clouds_config is called. + assert: InvalidCloudConfigError is raised. + """ + charm = factories.MockCharmFactory() + charm.config[state.OPENSTACK_PASSWORD_CONFIG_NAME] = "" + charm.config[state.OPENSTACK_PASSWORD_SECRET_CONFIG_NAME] = "" + + with pytest.raises(state.InvalidCloudConfigError) as exc: + state._parse_openstack_clouds_config(charm) + + assert "Please supply OpenStack password" in str(exc) + + # pylint: enable=undefined-variable,unused-variable From c89d2891efedc9488c4b40df114052f4c9bcc311 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 17 Apr 2026 06:28:03 +0000 Subject: [PATCH 05/13] fix: lint --- src/state.py | 6 ++++-- tests/unit/test_state.py | 7 +++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/state.py b/src/state.py index bc4d4773..5e46e69d 100644 --- a/src/state.py +++ b/src/state.py @@ -36,8 +36,10 @@ OPENSTACK_AUTH_URL_CONFIG_NAME = "openstack-auth-url" # Bandit thinks this is a hardcoded password OPENSTACK_PASSWORD_CONFIG_NAME = "openstack-password" # nosec: hardcoded_password_string -# nosec: hardcoded_password_string -OPENSTACK_PASSWORD_SECRET_CONFIG_NAME = "openstack-password-secret" +# Bandit thinks this is a hardcoded password +OPENSTACK_PASSWORD_SECRET_CONFIG_NAME = ( + "openstack-password-secret" # 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" diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index 1143ea39..cdf59c4a 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -7,6 +7,7 @@ # pylint:disable=protected-access import os +import secrets from unittest.mock import MagicMock import ops @@ -475,12 +476,13 @@ def test__parse_openstack_clouds_config_legacy_password(): assert: the clouds config is parsed correctly using the legacy password. """ charm = factories.MockCharmFactory() - charm.config[state.OPENSTACK_PASSWORD_CONFIG_NAME] = "legacy-password" + test_password = secrets.token_hex(16) + charm.config[state.OPENSTACK_PASSWORD_CONFIG_NAME] = test_password charm.config[state.OPENSTACK_PASSWORD_SECRET_CONFIG_NAME] = "" clouds_config = state._parse_openstack_clouds_config(charm) - assert clouds_config.clouds[state.CLOUD_NAME].auth.password == "legacy-password" + assert clouds_config.clouds[state.CLOUD_NAME].auth.password == test_password def test__parse_openstack_clouds_config_no_password(): @@ -702,3 +704,4 @@ def test__parse_script_secrets_from_config(secret: str, expected_secrets_map: di mock_charm.config = {state.SCRIPT_SECRET_CONFIG_NAME: secret} assert state._parse_script_secrets(charm=mock_charm) == expected_secrets_map + assert state._parse_script_secrets(charm=mock_charm) == expected_secrets_map From 843c3a7a5e8eebaeb09d3e164b18b6b5bb9ad148 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 20 Apr 2026 13:51:33 +0800 Subject: [PATCH 06/13] fix: address PR review comments for sensitive secret configs - charmcraft.yaml: remove trailing backslash in YAML literal block for openstack-password deprecation notice to avoid literal backslash in rendered description - docs/changelog.md: add PR link and date to changelog entry for consistency with other entries - src/state.py: remove unnecessary nosec comment on OPENSTACK_PASSWORD_SECRET_CONFIG_NAME (not a password literal); add Juju version check and secret ID format validation before calling model.get_secret() in _parse_openstack_clouds_config so users get clear error messages (upgrade guidance or expected format) - tests/integration/conftest.py: fix dict_keys vs set type annotation in _prepare_charmhub_app_config; add legacy openstack-password fallback when charmhub revision does not expose openstack-password-secret; fix grant order in app_on_charmhub_fixture (deploy without secret config, grant, then set_config to trigger config-changed after permissions are in place); import OPENSTACK_PASSWORD_CONFIG_NAME - tests/unit/factories.py: remove misleading nosec marker from secret ID value (it is a reference, not a credential) - tests/unit/test_state.py: add patch_juju_version_33 to tests using the secret path; add tests for unsupported Juju version and invalid secret ID format Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- charmcraft.yaml | 2 +- docs/changelog.md | 2 +- src/state.py | 17 ++++++++++++---- tests/integration/conftest.py | 37 +++++++++++++++++++++++++++++++---- tests/unit/factories.py | 2 +- tests/unit/test_state.py | 34 ++++++++++++++++++++++++++++++++ 6 files changed, 83 insertions(+), 11 deletions(-) diff --git a/charmcraft.yaml b/charmcraft.yaml index ebe42a76..f166394e 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -113,7 +113,7 @@ config: description: | The password section of the clouds.yaml contents, used to authenticate the OpenStack \ client (e.g. myverysecurepassword). See https://docs.openstack.org/python-openstackclient/\ - queens/configuration/index.html for more information. \ + queens/configuration/index.html for more information. DEPRECATED: Use openstack-password-secret instead for better security. openstack-password-secret: type: secret diff --git a/docs/changelog.md b/docs/changelog.md index 29147ceb..4d65d883 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,7 +1,7 @@ -## [#219 Use Juju secrets] +## [#219 Use Juju secrets](https://github.com/canonical/github-runner-image-builder-operator/pull/219) (2026-04-17) * Add new `openstack-password-secret` configuration option to securely store OpenStack passwords using Juju secrets. * Deprecated `openstack-password` configuration option (still supported for backward compatibility). * Users can migrate to the new secret-based approach by setting `openstack-password-secret` instead of `openstack-password`. diff --git a/src/state.py b/src/state.py index 5e46e69d..a714fa59 100644 --- a/src/state.py +++ b/src/state.py @@ -36,10 +36,7 @@ OPENSTACK_AUTH_URL_CONFIG_NAME = "openstack-auth-url" # Bandit thinks this is a hardcoded password OPENSTACK_PASSWORD_CONFIG_NAME = "openstack-password" # nosec: hardcoded_password_string -# Bandit thinks this is a hardcoded password -OPENSTACK_PASSWORD_SECRET_CONFIG_NAME = ( - "openstack-password-secret" # nosec: hardcoded_password_string -) +OPENSTACK_PASSWORD_SECRET_CONFIG_NAME = "openstack-password-secret" 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" @@ -645,6 +642,18 @@ def _parse_openstack_clouds_config(charm: ops.CharmBase) -> OpenstackCloudsConfi # Prefer the secret-based password if provided if password_secret_id: + juju_version = ops.JujuVersion.from_environ() + if juju_version < MIN_JUJU_VERSION_WITH_SECRET_SUPPORT: + raise InvalidCloudConfigError( + f"Secrets are not supported in Juju version {juju_version}. " + "Please consider upgrading the Juju controller to >= 3.3 or use " + "the openstack-password configuration option." + ) + if not password_secret_id.startswith("secret:"): + raise InvalidCloudConfigError( + f"Invalid value '{password_secret_id}' for openstack-password-secret. " + "Expected a Juju secret ID in the format 'secret:'." + ) try: secret = charm.model.get_secret(id=password_secret_id) except ops.SecretNotFoundError as exc: diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 81a35cda..06b43a13 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -37,6 +37,7 @@ EXTERNAL_BUILD_FLAVOR_CONFIG_NAME, EXTERNAL_BUILD_NETWORK_CONFIG_NAME, OPENSTACK_AUTH_URL_CONFIG_NAME, + OPENSTACK_PASSWORD_CONFIG_NAME, OPENSTACK_PASSWORD_SECRET_CONFIG_NAME, OPENSTACK_PROJECT_CONFIG_NAME, OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME, @@ -415,12 +416,16 @@ async def app_fixture( await test_configs.model.remove_application(app_name=app.name) -async def _prepare_charmhub_app_config(ops_test, app_config: dict) -> tuple[str, dict, set]: +async def _prepare_charmhub_app_config( + ops_test, app_config: dict, openstack_password: str +) -> tuple[str, dict, set[str]]: """Prepare the application config for charmhub deployment. Args: ops_test: The pytest operator test instance. app_config: The base application configuration. + openstack_password: The plaintext OpenStack password, used as a fallback when the + charmhub revision does not yet expose openstack-password-secret. Returns: A tuple of (channel, prepared_config, config_options). @@ -432,7 +437,7 @@ async def _prepare_charmhub_app_config(ops_test, app_config: dict) -> tuple[str, ) assert ret_code == 0, f"Failed to get charm info: {stderr}" charmhub_info = json.loads(stdout.strip()) - charmhub_config_options = charmhub_info["charm"]["config"]["Options"].keys() + charmhub_config_options = set(charmhub_info["charm"]["config"]["Options"].keys()) charmhub_app_config = {k: v for k, v in app_config.items() if k in charmhub_config_options} # We might need to test using the legacy config options. @@ -441,6 +446,14 @@ async def _prepare_charmhub_app_config(ops_test, app_config: dict) -> tuple[str, if (legacy_opt := f"{legacy_config_prefix}{opt}") in charmhub_config_options: charmhub_app_config[legacy_opt] = app_config[opt] + # If the charmhub revision doesn't expose openstack-password-secret yet, fall back to the + # legacy openstack-password option so the charm has credentials during initial deployment. + if ( + OPENSTACK_PASSWORD_SECRET_CONFIG_NAME not in charmhub_config_options + and OPENSTACK_PASSWORD_CONFIG_NAME in charmhub_config_options + ): + charmhub_app_config[OPENSTACK_PASSWORD_CONFIG_NAME] = openstack_password + return charmhub_channel, charmhub_app_config, charmhub_config_options @@ -451,23 +464,39 @@ async def app_on_charmhub_fixture( base_machine_constraint: str, ops_test, openstack_password_secret: _Secret, + private_endpoint_configs: PrivateEndpointConfigs, ) -> AsyncGenerator[Application, None]: """Fixture for deploying the charm from charmhub.""" # Normally we would use latest/stable, but upgrading # from stable is currently broken, and therefore we are using edge. Change this in the future. charmhub_channel, charmhub_app_config, charmhub_config_options = ( - await _prepare_charmhub_app_config(ops_test, app_config) + await _prepare_charmhub_app_config( + ops_test, app_config, private_endpoint_configs["password"] + ) ) + + # Deploy without the secret-backed config so the charm doesn't try to read the secret + # before the grant is in place. + deploy_config = { + k: v + for k, v in charmhub_app_config.items() + if k != OPENSTACK_PASSWORD_SECRET_CONFIG_NAME + } app: Application = await test_configs.model.deploy( "github-runner-image-builder", application_name=f"image-builder-operator-{test_configs.test_id}", constraints=base_machine_constraint, - config=charmhub_app_config, + config=deploy_config, channel=charmhub_channel, ) if OPENSTACK_PASSWORD_SECRET_CONFIG_NAME in charmhub_config_options: + # Grant access first, then set the config to trigger a config-changed hook + # after the charm already has read permissions for the secret. await app.model.grant_secret(openstack_password_secret.name, app.name) + await app.set_config( + {OPENSTACK_PASSWORD_SECRET_CONFIG_NAME: openstack_password_secret.id} + ) await test_configs.model.wait_for_idle(apps=[app.name], idle_period=30, timeout=60 * 30) diff --git a/tests/unit/factories.py b/tests/unit/factories.py index d3dfb08f..2a760a20 100644 --- a/tests/unit/factories.py +++ b/tests/unit/factories.py @@ -89,7 +89,7 @@ class Meta: # pylint: disable=too-few-public-methods EXTERNAL_BUILD_NETWORK_CONFIG_NAME: "test-network", OPENSTACK_AUTH_URL_CONFIG_NAME: "http://testing-auth/keystone", OPENSTACK_PASSWORD_CONFIG_NAME: "", - OPENSTACK_PASSWORD_SECRET_CONFIG_NAME: "secret:test-secret-id", # nosec: hardcoded_password_string + OPENSTACK_PASSWORD_SECRET_CONFIG_NAME: "secret:test-secret-id", OPENSTACK_PROJECT_DOMAIN_CONFIG_NAME: "test-project-domain", OPENSTACK_PROJECT_CONFIG_NAME: "test-project-name", OPENSTACK_USER_DOMAIN_CONFIG_NAME: "test-user-domain", diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index cdf59c4a..3573161e 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -396,6 +396,7 @@ def test__parse_openstack_clouds_config_invalid(missing_config: str): assert "Please supply all OpenStack configurations." in str(exc) +@pytest.mark.usefixtures("patch_juju_version_33") def test__parse_openstack_clouds_config(): """ arrange: given mock charm from factory. @@ -437,6 +438,7 @@ def test__parse_openstack_clouds_config(): ), ], ) +@pytest.mark.usefixtures("patch_juju_version_33") def test__parse_openstack_clouds_config_secret_error(exception: Exception, expected_error: str): """ arrange: given a mocked model get_secret method that raises a given error. @@ -452,6 +454,7 @@ def test__parse_openstack_clouds_config_secret_error(exception: Exception, expec assert expected_error in str(exc) +@pytest.mark.usefixtures("patch_juju_version_33") def test__parse_openstack_clouds_config_missing_password_key(): """ arrange: given a secret that does not contain a 'password' key. @@ -501,6 +504,37 @@ def test__parse_openstack_clouds_config_no_password(): assert "Please supply OpenStack password" in str(exc) +@pytest.mark.usefixtures("patch_juju_version_29") +def test__parse_openstack_clouds_config_secret_unsupported_juju(): + """ + arrange: given a charm with openstack-password-secret set but running on Juju < 3.3. + act: when _parse_openstack_clouds_config is called. + assert: InvalidCloudConfigError is raised with guidance to upgrade or use legacy option. + """ + charm = factories.MockCharmFactory() + + with pytest.raises(state.InvalidCloudConfigError) as exc: + state._parse_openstack_clouds_config(charm) + + assert "openstack-password" in str(exc) + + +@pytest.mark.usefixtures("patch_juju_version_33") +def test__parse_openstack_clouds_config_invalid_secret_id_format(): + """ + arrange: given a charm with openstack-password-secret set to a non-secret-id value. + act: when _parse_openstack_clouds_config is called. + assert: InvalidCloudConfigError is raised with guidance on the expected format. + """ + charm = factories.MockCharmFactory() + charm.config[state.OPENSTACK_PASSWORD_SECRET_CONFIG_NAME] = "not-a-secret-id" + + with pytest.raises(state.InvalidCloudConfigError) as exc: + state._parse_openstack_clouds_config(charm) + + assert "secret:" in str(exc) + + # pylint: enable=undefined-variable,unused-variable From c2269ac6050819b0d8f66b621e134b9949f83750 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Apr 2026 06:11:43 +0000 Subject: [PATCH 07/13] fix: remove Juju version gate for OpenStack password secrets Agent-Logs-Url: https://github.com/canonical/github-runner-image-builder-operator/sessions/453107d7-866a-4c44-8000-d97b9348860b Co-authored-by: yanksyoon <37652070+yanksyoon@users.noreply.github.com> --- src/state.py | 7 ------- tests/unit/test_state.py | 15 --------------- 2 files changed, 22 deletions(-) diff --git a/src/state.py b/src/state.py index a714fa59..db932005 100644 --- a/src/state.py +++ b/src/state.py @@ -642,13 +642,6 @@ def _parse_openstack_clouds_config(charm: ops.CharmBase) -> OpenstackCloudsConfi # Prefer the secret-based password if provided if password_secret_id: - juju_version = ops.JujuVersion.from_environ() - if juju_version < MIN_JUJU_VERSION_WITH_SECRET_SUPPORT: - raise InvalidCloudConfigError( - f"Secrets are not supported in Juju version {juju_version}. " - "Please consider upgrading the Juju controller to >= 3.3 or use " - "the openstack-password configuration option." - ) if not password_secret_id.startswith("secret:"): raise InvalidCloudConfigError( f"Invalid value '{password_secret_id}' for openstack-password-secret. " diff --git a/tests/unit/test_state.py b/tests/unit/test_state.py index 3573161e..6d0572ec 100644 --- a/tests/unit/test_state.py +++ b/tests/unit/test_state.py @@ -504,21 +504,6 @@ def test__parse_openstack_clouds_config_no_password(): assert "Please supply OpenStack password" in str(exc) -@pytest.mark.usefixtures("patch_juju_version_29") -def test__parse_openstack_clouds_config_secret_unsupported_juju(): - """ - arrange: given a charm with openstack-password-secret set but running on Juju < 3.3. - act: when _parse_openstack_clouds_config is called. - assert: InvalidCloudConfigError is raised with guidance to upgrade or use legacy option. - """ - charm = factories.MockCharmFactory() - - with pytest.raises(state.InvalidCloudConfigError) as exc: - state._parse_openstack_clouds_config(charm) - - assert "openstack-password" in str(exc) - - @pytest.mark.usefixtures("patch_juju_version_33") def test__parse_openstack_clouds_config_invalid_secret_id_format(): """ From 04a208eb4ce0027a8921a0b73448a50b208baf50 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 20 Apr 2026 15:02:06 +0800 Subject: [PATCH 08/13] test: update secret passing --- tests/conftest.py | 14 ++++++++++++-- tox.ini | 10 ++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 96f5e0d2..5d157ff6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,6 +3,8 @@ """Fixtures for github runner charm.""" +import os + from pytest import Parser @@ -29,52 +31,60 @@ def pytest_addoption(parser: Parser): "--openstack-network-name-amd64", action="store", help="The Openstack network to create testing instances under.", + default=os.getenv("OPENSTACK_NETWORK_NAME_AMD64"), ) parser.addoption( "--openstack-flavor-name-amd64", action="store", help="The Openstack flavor to create testing instances with.", + default=os.getenv("OPENSTACK_FLAVOR_NAME_AMD64"), ) parser.addoption( "--openstack-auth-url-amd64", action="store", help="The URL to Openstack authentication service, i.e. keystone.", + default=os.getenv("OPENSTACK_AUTH_URL_AMD64"), ) parser.addoption( "--openstack-project-domain-name-amd64", action="store", help="The Openstack project domain name to use.", + default=os.getenv("OPENSTACK_PROJECT_DOMAIN_NAME_AMD64"), ) parser.addoption( "--openstack-project-name-amd64", action="store", help="The Openstack project name to use.", + default=os.getenv("OPENSTACK_PROJECT_NAME_AMD64"), ) parser.addoption( "--openstack-user-domain-name-amd64", action="store", help="The Openstack user domain name to use.", + default=os.getenv("OPENSTACK_USER_DOMAIN_NAME_AMD64"), ) parser.addoption( "--openstack-username-amd64", action="store", help="The Openstack user to authenticate as.", + default=os.getenv("OPENSTACK_USERNAME_AMD64"), ) parser.addoption( "--openstack-region-name-amd64", action="store", help="The Openstack region to authenticate to.", + default=os.getenv("OPENSTACK_REGION_NAME_AMD64"), ) # Shared private endpoint options parser.addoption( "--proxy", action="store", help="The HTTP proxy URL to apply on the Openstack runners.", - default=None, + default=os.getenv("PROXY"), ) parser.addoption( "--no-proxy", action="store", help="The no proxy URL(s) to apply on the Openstack runners.", - default=None, + default=os.getenv("NO_PROXY"), ) diff --git a/tox.ini b/tox.ini index 7cda05cf..cdff0cc9 100644 --- a/tox.ini +++ b/tox.ini @@ -148,6 +148,16 @@ pass_env = PYTEST_ADDOPTS OPENSTACK_PASSWORD_AMD64 OPENSTACK_PASSWORD_ARM64 + OPENSTACK_FLAVOR_NAME_AMD64 + OPENSTACK_NETWORK_NAME_AMD64 + OPENSTACK_AUTH_URL_AMD64 + OPENSTACK_PROJECT_DOMAIN_NAME_AMD64 + OPENSTACK_PROJECT_NAME_AMD64 + OPENSTACK_USER_DOMAIN_NAME_AMD64 + OPENSTACK_USERNAME_AMD64 + OPENSTACK_REGION_NAME_AMD64 + PROXY + NO_PROXY deps = -r{[vars]tst_path}integration/requirements.txt -r{toxinidir}/requirements.txt From 6be3931b2f13e7acfba6adcbcf2f06a97c15e75a Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 20 Apr 2026 15:16:03 +0800 Subject: [PATCH 09/13] fix: restore nosec comments for bandit compatibility Bandit flags 'openstack-password-secret' as B105 (hardcoded_password_string) because the string contains 'password'. Restore the nosec comment on OPENSTACK_PASSWORD_SECRET_CONFIG_NAME in state.py. Also restore the nosec comment in factories.py for the mock secret content dict, which bandit flags as B105 in full-repo scan mode. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/state.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/state.py b/src/state.py index db932005..cd11b230 100644 --- a/src/state.py +++ b/src/state.py @@ -36,7 +36,10 @@ OPENSTACK_AUTH_URL_CONFIG_NAME = "openstack-auth-url" # Bandit thinks this is a hardcoded password OPENSTACK_PASSWORD_CONFIG_NAME = "openstack-password" # nosec: hardcoded_password_string -OPENSTACK_PASSWORD_SECRET_CONFIG_NAME = "openstack-password-secret" +# Bandit thinks this is a hardcoded password +OPENSTACK_PASSWORD_SECRET_CONFIG_NAME = ( + "openstack-password-secret" # 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" From 62ad4206da1255308a72dc6aeb53f46ab2bb498a Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 20 Apr 2026 15:24:11 +0800 Subject: [PATCH 10/13] fix: black formatting and pylint args in conftest - Collapse dict comprehension to single line to satisfy black formatting - Collapse set_config call to single line to satisfy black formatting - Add pylint disable for too-many-arguments and too-many-positional-arguments on app_on_charmhub_fixture which now takes 6 fixture parameters Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/integration/conftest.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 06b43a13..450b4506 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -458,7 +458,7 @@ async def _prepare_charmhub_app_config( @pytest_asyncio.fixture(scope="module", name="app_on_charmhub") -async def app_on_charmhub_fixture( +async def app_on_charmhub_fixture( # pylint: disable=too-many-arguments,too-many-positional-arguments test_configs: TestConfigs, app_config: dict, base_machine_constraint: str, @@ -478,9 +478,7 @@ async def app_on_charmhub_fixture( # Deploy without the secret-backed config so the charm doesn't try to read the secret # before the grant is in place. deploy_config = { - k: v - for k, v in charmhub_app_config.items() - if k != OPENSTACK_PASSWORD_SECRET_CONFIG_NAME + k: v for k, v in charmhub_app_config.items() if k != OPENSTACK_PASSWORD_SECRET_CONFIG_NAME } app: Application = await test_configs.model.deploy( "github-runner-image-builder", @@ -494,9 +492,7 @@ async def app_on_charmhub_fixture( # Grant access first, then set the config to trigger a config-changed hook # after the charm already has read permissions for the secret. await app.model.grant_secret(openstack_password_secret.name, app.name) - await app.set_config( - {OPENSTACK_PASSWORD_SECRET_CONFIG_NAME: openstack_password_secret.id} - ) + await app.set_config({OPENSTACK_PASSWORD_SECRET_CONFIG_NAME: openstack_password_secret.id}) await test_configs.model.wait_for_idle(apps=[app.name], idle_period=30, timeout=60 * 30) From f51137c125f8895cb45fc3389ca8581ccdb3f4cc Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 20 Apr 2026 15:51:22 +0800 Subject: [PATCH 11/13] fix: remove encoding kwarg from basicConfig with handlers mypy rejects encoding= when handlers= is also passed to basicConfig, since only the filename-based overload accepts encoding. The encoding is already set on the WatchedFileHandler constructor, so removing it from basicConfig is correct and harmless. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- app/src/github_runner_image_builder/logging.py | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/github_runner_image_builder/logging.py b/app/src/github_runner_image_builder/logging.py index 771da30e..b3fa134c 100644 --- a/app/src/github_runner_image_builder/logging.py +++ b/app/src/github_runner_image_builder/logging.py @@ -27,5 +27,4 @@ def configure(log_level: str | int) -> None: logging.basicConfig( level=log_level_normalized, handlers=(log_handler,), - encoding="utf-8", ) From 3aa52e2671e3e234f70c89d7af035c821db6f04d Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 20 Apr 2026 16:24:46 +0800 Subject: [PATCH 12/13] ci: trigger ci From 5cfba7515db5e998720b60f97a4648a4c7895b7c Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 20 Apr 2026 17:48:39 +0800 Subject: [PATCH 13/13] ci: trigger ci