Skip to content

feat: pass a configured OpenStack security group to runner scale sets (ISD-7035) - #440

Open
yanksyoon wants to merge 3 commits into
mainfrom
feat/isd-7035-security-group-id
Open

yanksyoon wants to merge 3 commits into
mainfrom
feat/isd-7035-security-group-id

Conversation

@yanksyoon

@yanksyoon yanksyoon commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

What this PR does

Lets the operator pin the OpenStack security group a GARM scale set's runners are launched in, by passing an ID or name straight through to the provider.

garm-configurator                        garm                            provider / Nova
─────────────────                        ────                            ───────────────
security-group: sg-runner
        └── relation: security_group ──► security_groups extra spec ──► runner port
  • garm-configurator: new optional security-group configuration. It is published on the garm-configurator relation as security_group, like image_visibility.
  • garm: reads security_group from the configurator's databag and puts it in the scale set's extra_specs.security_groups, which garm-provider-openstack (v0.1.5, already pinned in the rock) maps to the instance port. A change is picked up by the existing drift detection, so the scale set is updated in place.

Why we need it

Runners in one OpenStack project share its default security group, so they can reach each other, and a compromised job can attack its peers (ISD-7035). Pinning a locked-down group isolates them. The group itself stays the operator's to create and manage: the charms only pass the reference through.

This is the minimal version of ISD-7035. It does not create or converge the group. #436 manages a group's lifecycle from the charms; it is a separate, larger change and this PR neither builds on it nor modifies it. If both land, security-group can name the group #436 manages.

Test plan

  • Unit tests (tox -c tox.toml -e unit, per charm): garm 454 passed, garm-configurator 66 passed. The new tests assert the configured group reaches the provider:
    • test_create_sends_security_group_as_an_extra_spec — the create call carries extra_specs == {"security_groups": ["sg-runner"]}.
    • test_update_converges_security_group — a changed or removed group converges in one update instead of re-updating on every reconcile.
    • test_security_group_follows_the_configurator_databag — the databag value lands on the scale set spec, and an absent value stays unset for an older configurator.
    • test_security_group_written_to_garm_configurator_relation — the charm config reaches the databag, empty when unset.
  • fmt, lint, complexity, static: clean for both charms.
  • Not covered by an integration test: attaching the group needs a real Neutron group to exist, which the tests do not model. tox -e garm-e2e against a tenant would be the check.

Review focus

  • The group is read with str(charm.config.get(..., "")).strip() rather than _get_optional_string_config(...) plus or "": ScalesetConfig.from_charm and _configure_garm_relation both sit exactly at the cognitive-complexity limit, and either form pushes one of them over. That is why the field defaults to "" instead of None.
  • The extra spec is a list (security_groups), matching garm-provider-openstack, but the configurator takes one group. A list-valued option is the obvious next step if operators need more than one.

Potential breaking changes

None. The option is optional; while unset the charm publishes an empty security_group and the scale set's extra specs are unchanged, so the provider keeps attaching the project's default group.

New dependencies, APIs, modules, or workflow changes

  • New charm option: garm-configurator security-group.
  • New relation databag key: security_group on the garm-configurator relation.
  • No new dependencies. security_groups is an existing garm-provider-openstack extra spec.

Checklist

  • Changes comply with the project's coding standards and guidelines (see CONTRIBUTING.md and STYLE.md)
  • CONTRIBUTING.md has been updated upon changes to the contribution/development process (n/a: no process change)
  • Technical author has been assigned to review the PR in case of documentation changes (usually *.md files)
  • I updated docs/changelog.md with user-relevant changes
  • I used AI to assist with preparing this PR
  • I added or updated tests as needed (unit and integration)
  • If integration test modules are used: (n/a: no integration module change)
  • If this PR involves a Grafana dashboard: (n/a)
  • If this PR involves Terraform: (n/a: the module takes a generic config map)
  • If this PR involves Rockcraft: (n/a)
  • If this PR adds/removes a charm, or changes a charm's base class, conventions, tooling, or repo structure: (n/a)
  • If this PR changes .copilot-collections.yaml or .github/instructions/: (n/a)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant