Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v15] integrations/operator: Add openSSH and openSSHEICE servers #38478

Merged
merged 7 commits into from
Mar 27, 2024

Conversation

hugoShaka
Copy link
Contributor

@hugoShaka hugoShaka commented Feb 21, 2024

Backport #37651 (OpenSSH support) to branch/v15

also

Backport #38992 (label support) to branch/v15

changelog: Add OpensshServer and OpensshEICEServer support in the operator

Changelog: The operator now supports setting labels on Teleport resources. Labels are copied from the Kubernetes CR.

@public-teleport-github-review-bot

@hugoShaka - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes.

@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from zmb3 March 1, 2024 17:16
hugoShaka and others added 6 commits March 7, 2024 16:54
…abels (#38992)

* Refactor operator resource reconciler to support resource153 + add label propagation

I initially just wanted to add label propagation (which is required for
OpenSSH and OpenSSHEICE servers).

However I ran into an issue: not all Teleport resources implement the
`ResourceWithLabels` interface.  Basically we have 4 cases:
- resources implementing `ResourceWithLabels` (e.g. User, Role, ...)
- resources implementing `ResourceWithOrigin` but not
  `ResourceWithLabels` (e.g. GitHub connector, ProvisionToken, ...)
- resources that have been tweaked to implement only the operator
  `TeleportResource` interface (`LoginRule`)
- resources implementing `Resource153`

To support all those resoucres, we can either:
- edit each resource so they expose the same methods (e.g.
  ProvisionToken exposes `GetLabels` instead of `GetStaticLabels`)
- wrap each ressource in a wrapper that implements the operator
  TeleportResource interface
- add multiple reconcilers, one for each resource kind

I don't think it's sane for the operator to force changes on Teleport
resources. Especially as those are known to be non consistent. I also
don't want the operator to maintain a copy of each resource, this is not
viable on the long term.

For those reasons I chose the 3rd approach and built 3 reconcilers. Then
I deducplicated the code to end up with a generic reconciler that can be
tuned by passing an adapter. The adapter is an empty struct that has all
the methods to extract the information the operator requires from the
resource.

The resulting PR is large and heavily uses generics but the amount of
logical changes is quite low (the only diff is that we now copy labels).
I can walk you through the PR if needed.

With this PR, the operator supports properly any resource implementing
`types.Resource153`, this will be useful to add `TeleportBotV1` support.

* Update integrations/operator/controllers/reconcilers/base.go

Co-authored-by: Noah Stride <[email protected]>

* use structured logging

---------

Co-authored-by: Noah Stride <[email protected]>
@hugoShaka hugoShaka force-pushed the bot/backport-37651-branch/v15 branch from 1df1ada to 9766732 Compare March 7, 2024 21:55
@hugoShaka hugoShaka enabled auto-merge March 14, 2024 14:59
@hugoShaka hugoShaka added this pull request to the merge queue Mar 14, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 14, 2024
@hugoShaka hugoShaka added this pull request to the merge queue Mar 14, 2024
@hugoShaka hugoShaka removed this pull request from the merge queue due to a manual request Mar 14, 2024
@hugoShaka hugoShaka added this pull request to the merge queue Mar 27, 2024
Merged via the queue into branch/v15 with commit 8e98203 Mar 27, 2024
35 checks passed
@hugoShaka hugoShaka deleted the bot/backport-37651-branch/v15 branch March 27, 2024 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants