Skip to content

Support reconciling infrastructure without ssh key - #319

Merged
ske-prow[bot] merged 2 commits into
mainfrom
fix-no-ssh-access
Oct 12, 2026
Merged

Support reconciling infrastructure without ssh key#319
ske-prow[bot] merged 2 commits into
mainfrom
fix-no-ssh-access

Conversation

@MichaelEischer

@MichaelEischer MichaelEischer commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

How to categorize this PR?

What this PR does / why we need it:

Gardener supports disabling sshAccess for nodes. This results in Gardener deleting the ssh keypair. As a consequence the gardener-extension-provider-... and machine-controller-manager-provider-... need to work even if no keypair is present.

The flow in ensureStackitSSHKeyPair and ensureSSHKeyPair is already able to delete a changed keypair. If the keypair is removed from the infrastructure object, this already causes the above code path to trigger and therefore ensure a cleanup of the keypair. However, currently the validation in CreateKeypair results in an error for an empty keypair. Thus, skip that create call if no keypair should exist and set the keypairname in the state to an empty string.

The machine-controller-manager-provider-... will take that as a signal to create machines without ssh key. machine-controller-manager-provider-stackit already supports this, whereas machine-controller-manager-provider-openstack needs a fix, see gardener/machine-controller-manager-provider-openstack#419.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

/hold depends on gardener/machine-controller-manager-provider-openstack#419 to fully work.

I'm somewhat undecided whether we want to integration test the full route including testing that the old keypair gets deleted.

@ske-prow ske-prow Bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 2, 2026
@MichaelEischer
support reconciling infrastructure without ssh key
875e18e 
Gardener deletes the ssh key for a shoot when disabling ssh access via
`Shoot.spec.provider.workersSettings.sshAccess.enabled=false`. Thus,
reconciling the infrastructure object must work without an ssh key being
present.
@MichaelEischer
test that infrastructure can be created without ssh key
6c7b4d5
@dergeberl
dergeberl requested a review from a team October 2, 2026 13:51
@MichaelEischer MichaelEischer added the kind/enhancement Enhancement, improvement, extension label Oct 2, 2026
@ske-prow ske-prow Bot removed the do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. label Oct 2, 2026
@MichaelEischer

MichaelEischer commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Tested the whole 2x2 matrix of openstack/stackit infrastructure and mcm provider. Using this PR and gardener/machine-controller-manager-provider-openstack#419 .

  • Shoot creation with sshAccess disabled
  • Reenabled ssh. ssh-keys were successfully created (openstack & stackit APIs). Recreated all machines afterwards, which also worked.
  • Disabled ssh access again. Checked that ssh keys were removed (openstack & stackit APIs). Recreated all machines
  • Deleted all shoots while no ssh keys existed. Also worked correctly.

@dergeberl

dergeberl commented Oct 5, 2026

Copy link
Copy Markdown
Member

/lgtm
should we wait until gardener/machine-controller-manager-provider-openstack#419 ?
Because it would at least already work with the STACKIT MCM?

@ske-prow ske-prow Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 5, 2026
@ske-prow

ske-prow Bot commented Oct 5, 2026

Copy link
Copy Markdown

LGTM label has been added.

DetailsGit tree hash: a1e9eeaf600555b740463b27b08f86003ce79b87

@MichaelEischer

MichaelEischer commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Yes, the stackit mcm part already works. But it's probably a good idea to wait for the mcm-openstack PR.

@dergeberl

dergeberl commented Oct 12, 2026

Copy link
Copy Markdown
Member

/unhold
mcm is updated #334

@ske-prow ske-prow Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 12, 2026
dergeberl
dergeberl approved these changes Oct 12, 2026
View reviewed changes

@dergeberl dergeberl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@ske-prow

ske-prow Bot commented Oct 12, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dergeberl

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ske-prow ske-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 12, 2026
@ske-prow
ske-prow Bot merged commit 10d76a8 into main Oct 12, 2026
4 checks passed
@ske-prow
ske-prow Bot deleted the fix-no-ssh-access branch October 12, 2026 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dergeberl dergeberl dergeberl approved these changes

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@MichaelEischer @dergeberl