* Removes the entire notion of a DynamicAccessList from Teleport
Insurmountable security issues were found with the notion of dynamic
AccessList ownership and membership, so the entire concept is being
removed.
This PR removes all internal usage of the Dynamic access list
concept. The corresponding protobuf updates are coming in a later
PR.
Changelog: Removed Dynamic AccessList
* Fix API tests
* More API test fixes
* Fix integrations tests
* 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 <noah.stride@goteleport.com>
* use structured logging
---------
Co-authored-by: Noah Stride <noah.stride@goteleport.com>
* integrations/operator: Add openSSH and openSSHEICE servers
* lint
* Update integrations/operator/controllers/resources/openssheiceserverv2_controller.go
Co-authored-by: Marco André Dinis <marco.dinis@goteleport.com>
* rename Openssh to OpenSSH and fix comments
* hide cmdlabels
---------
Co-authored-by: Marco André Dinis <marco.dinis@goteleport.com>
The commit contains the following changes:
- add multi-version support by creating new kinds. This is currently done via an override to maintain compatibility. At some point, we might migrate unversioned kinds (TeleportRole) to versioned kinds (TeleportRoleV5) and remove the override
- unify all resource statuses in a single struct. This simplifies the CRD building because we are not trying to guess which package is correct anymore when parsing go's AST
- update crdgen snapshots
This PR does not update the base CRDs in the operator or the Hlem chart. This will happen in the next PR, adding go types and controller support.
Note for reviewers: the changeset might seem large, but the changes are split into commits that are much more reviewable. You only need to review the first two commits.
This commit prepares everything for us to switch to a single reconciler for all resources. Part of https://github.com/gravitational/teleport/issues/21416
This commit does the following changes:
- setup controllers for the unstructured objects instead of the structured ones (like `Role` was doing). This is due to the bad behaviour of `utils.Strings` of serializing as a single string if the list contains a single element.
- use a single scheme for everything instead of multiple ones. We suffered from drifts between main and tests in the past.
- fetch the GVK for the unstructure objects from the scheme. This operation can fail and now a bunch of builder/init functions can return errors
- Introduce a new `Reconciler` interface to hide the controller internals
- unify how controllers are started in main and tests. This is not directly related to the PR goal (preparing the ground for a unified controller) but I was drawn into this when I made the constructors return errors.
This commit creates a standard set of compare options for all tests that
handles the common use cases:
- ignore ID, labels and other dynamic metadata
- apply sane time comparison by default
- equates nil maps and empty maps (caused by proto serialization)
This solves a couple of comparison issues I faced in another PR.
While the api client already had support for the new upsert method
it was unimplemented server side and would always fallback to the
legacy UpsertOIDCConnector RPC. This implements the RPC handler
and updates the signature of UpsertOIDCConnector to return the
upserted connector in the same manner that Create and Update do.
While the api client already had support for the new upsert method
it was unimplemented server side and would always fallback to the
legacy UpsertSAMLConnector RPC. This implements the RPC handler
and updates the signature of UpsertSAMLConnector to return the
upserted connector in the same manner that Create and Update do.
While the api client already had support for the new upsert method
it was unimplemented server side and would always fallback to the
legeacy UpsertGithubConnector RPC. This implements the RPC handler
and updates the signature of UpsertGithubConnector to return the
upserted connector in the same manner that Create and Update do.
* use eventuallyWithT
* integrations/operator: remove require checks from Eventually funcs
* fixup! integrations/operator: remove require checks from Eventually funcs
* Fix Teleport update reconciliation on `status` updates
This pull request addresses the issue where the Teleport operator reconciliation runs every time the operator updates the `status` subresource.
This continuous reconciliation has led to an infinite loop, causing millions of reconciliations per minute.
When an error occurs, such as having invalid role properties, the Operator updates the status and returns an error, which should trigger a rescheduled reconciliation with exponential backoff. The problem arises because the operator failed to enforce a resource generation change, resulting in an immediate trigger of a new reconciliation when the `status` field is updated.
This pull request modifies the operator to avoid updating subresources and only trigger updates when there is a change in resource generation.
Special thanks to @strideynet for confirming my hypothesis and giving
the solution!
Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
* return proper status conditions on failures
* enforce condition update on silentUpdateStatus
---------
Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
In addition to adding server and backend handling for create and
update roles, the services.Access interface was updated to return
a role from the existing Create/UpsertRole methods. Bumps the e
ref to incorporate the associated changes needed there to prevent
breaking the build.
services.UsersService now takes a context and returns the user
from write operations as shown in the diff below. The bulk of the
changes are from modifying code to account for the additional
parameter and/or return value. Functional changes to better make
use of the new API will come in follow up PRs.
```diff
// UserGetter is responsible for getting users
type UserGetter interface {
// GetUser returns a user by name
- GetUser(user string, withSecrets bool) (types.User, error)
+ GetUser(ctx context.Context, user string, withSecrets bool) (types.User, error)
}
// UsersService is responsible for basic user management
type UsersService interface {
UserGetter
// CreateUser creates user, only if the user entry does not exist
- CreateUser(user types.User) error
+ CreateUser(ctx context.Context, user types.User) (types.User, error)
// UpdateUser updates an existing user.
- UpdateUser(ctx context.Context, user types.User) error
+ UpdateUser(ctx context.Context, user types.User) (types.User, error)
// UpdateAndSwapUser reads an existing user, runs `fn` against it and writes
// the result to storage. Return `false` from `fn` to avoid storage changes.
// Roughly equivalent to [GetUser] followed by [CompareAndSwapUser].
// Returns the storage user.
UpdateAndSwapUser(ctx context.Context, user string, withSecrets bool, fn func(types.User) (changed bool, err error)) (types.User, error)
// UpsertUser updates parameters about user
- UpsertUser(user types.User) error
+ UpsertUser(ctx context.Context, user types.User) (types.User, error)
// CompareAndSwapUser updates an existing user, but fails if the user does
// not match an expected backend value.
CompareAndSwapUser(ctx context.Context, new, existing types.User) error
// DeleteUser deletes a user with all the keys from the backend
DeleteUser(ctx context.Context, user string) error
// GetUsers returns a list of users registered with the local auth server
- GetUsers(withSecrets bool) ([]types.User, error)
+ GetUsers(ctx context.Context, withSecrets bool) ([]types.User, error)
// DeleteAllUsers deletes all users
- DeleteAllUsers() error
+ DeleteAllUsers(ctx context.Context) error
}
```
Depends on gravitational/teleport.e#2346
Implements step 3 of #32949
* Add support for Protobuf Enums into Operator CRDs
This PR marks the Teleport enum fields as integer or string values. The
integer option is to ensure we are backwards compatibile with
previously installed CRDs.
Users can now represent their roles in Kubernetes custom resources and
refer enum fields as strings while their protobuf wire type is int32.
Fixes#29686
* add tests
* fix unit test
* integrations/crdgen: Support loading instructions from a file for easier debugging
* integrations/crdgen: Fix CRD generation for nested message declarations
* integrations/crdgen: Update CRDs and CRD snaphots after crdgen fix
* integrations/operator: Add a test covering machineID on GHA tokens
* document how to dump protoc requests
* k8s operator supports Okta import rules.
The k8s operator now supports Okta import rules, which will allow users to
use native k8s CRDs to provision Okta import rules in k8s environments.
* Fix helm test.
* Remove unneeded proto files.
* Use additionalProperties in the okta import rules CRD instead of properties.
* integrations/operator: Use a dedicated scheme in tests
Relying on a global scheme can cause race-conditions when registering
schema in multiple tests. This commit does two things in an attempt to
fix https://github.com/gravitational/teleport/issues/26753:
- use a separate Scheme for tests to avoid being affected by what others
might be doing with the scheme
- initiate the Scheme only once during import
* unexport scheme
This PR extends Teleport capabilities of `kubectl auth can-i` to support
evaluation of `kubernetes_resources`.
It also changes the error message to be clear to the users what's the
cause of the error and adds a warning when users try to edit invalid
roles.
* Add type for GitLab ProvisionToken
* Add default behaviour for domain
* Add IDTokenClaims for GitLab
* Add gitlab token source and token validator
* Thread GitLab support through auth and tbot packages
* Adjust cluster name fetching in token validator
* Initialize GitLab token validator in auth
* Improve comment on `sub`
* Working GitLab CI delegated joining
* Add additional token rule fields
* Add checking for new configuration fields
* add additional test cases for validation of gitlab config struct
* Add TestAuth_RegisterUsingToken_GitLab
* Add tests for IDTokenSource
* Fix imports
* Add tests for GitLab Token Validator
* Fix some comments that were incomplete
* Add license headers
* Move configuration from lib/service to lib/service/servicecfg
The new servicecfg package will hold only configuration for services.
This will allow other packages (like tctl and tsh) to depend on
servicecfg without pulling in all of lib/service (which has a number
of platform-specific details).
This is the first step towards being able to build tctl for Windows.
* Move PAM and BPF config into servicecfg
This breaks a compile-time dependency on BPF/PAM for tctl.
Trent Clarke