From a11e1527f09a157955258471a68deb2f995633cb Mon Sep 17 00:00:00 2001 From: "Benjamin A. Petersen" Date: Tue, 26 Mar 2024 12:51:33 -0400 Subject: [PATCH] Add github-upstream-observer Controller --- .../github_upstream_watcher.go | 154 ++++++++++++++++++ .../github_upstream_watcher_test.go | 4 + .../dynamic_upstream_idp_provider.go | 18 +- .../upstreamprovider/upsteam_provider.go | 4 + internal/supervisor/server/server.go | 13 ++ internal/upstreamgithub/upstreamgithub.go | 29 ++++ .../upstreamgithub/upstreamgithub_test.go | 4 + 7 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go create mode 100644 internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go create mode 100644 internal/upstreamgithub/upstreamgithub.go create mode 100644 internal/upstreamgithub/upstreamgithub_test.go diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go new file mode 100644 index 000000000..7a333c4a3 --- /dev/null +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go @@ -0,0 +1,154 @@ +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package githubupstreamwatcher implements a controller which watches GitHubIdentityProviders. +package githubupstreamwatcher + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + corev1informers "k8s.io/client-go/informers/core/v1" + + "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" + supervisorclientset "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned" + idpinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions/idp/v1alpha1" + pinnipedcontroller "go.pinniped.dev/internal/controller" + "go.pinniped.dev/internal/controller/conditionsutil" + "go.pinniped.dev/internal/controllerlib" + "go.pinniped.dev/internal/federationdomain/upstreamprovider" + "go.pinniped.dev/internal/upstreamgithub" +) + +const ( + // Setup for the name of our controller in logs. + controllerName = "github-upstream-observer" + + // Constants related to the client credentials Secret. + gitHubClientSecretType corev1.SecretType = "secrets.pinniped.dev/github-client" + + // // fixes lint to split from above group where const has explicit type + // clientIDDataKey = "clientID" + // clientSecretDataKey = "clientSecret" + // + // // Constants related to conditions. + // typeClientCredentialsValid = "ClientCredentialsValid" //nolint:gosec // this is not a credential. +) + +// UpstreamGitHubIdentityProviderICache is a thread safe cache that holds a list of validated upstream GitHub IDP configurations. +type UpstreamGitHubIdentityProviderICache interface { + SetGitHubIdentityProviders([]upstreamprovider.UpstreamGithubIdentityProviderI) +} + +type gitHubWatcherController struct { + cache UpstreamGitHubIdentityProviderICache + log logr.Logger + client supervisorclientset.Interface + gitHubIdentityProviderInformer idpinformers.GitHubIdentityProviderInformer + secretInformer corev1informers.SecretInformer +} + +// New instantiates a new controllerlib.Controller which will populate the provided UpstreamGitHubIdentityProviderICache. +func New( + idpCache UpstreamGitHubIdentityProviderICache, + client supervisorclientset.Interface, + gitHubIdentityProviderInformer idpinformers.GitHubIdentityProviderInformer, + secretInformer corev1informers.SecretInformer, + log logr.Logger, + withInformer pinnipedcontroller.WithInformerOptionFunc, +) controllerlib.Controller { + c := gitHubWatcherController{ + cache: idpCache, + client: client, + log: log.WithName(controllerName), + gitHubIdentityProviderInformer: gitHubIdentityProviderInformer, + secretInformer: secretInformer, + } + + return controllerlib.New( + controllerlib.Config{Name: controllerName, Syncer: &c}, + withInformer( + gitHubIdentityProviderInformer, + pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), + withInformer( + secretInformer, + pinnipedcontroller.MatchAnySecretOfTypeFilter(gitHubClientSecretType, pinnipedcontroller.SingletonQueue()), + controllerlib.InformerOption{}, + ), + ) +} + +// Sync implements controllerlib.Syncer. +func (c *gitHubWatcherController) Sync(ctx controllerlib.Context) error { + actualUpstreams, err := c.gitHubIdentityProviderInformer.Lister().List(labels.Everything()) + if err != nil { + return fmt.Errorf("failed to list GitHubIdentityProviders: %w", err) + } + + requeue := false + validatedUpstreams := make([]upstreamprovider.UpstreamGithubIdentityProviderI, 0, len(actualUpstreams)) + for _, upstream := range actualUpstreams { + valid := c.validateUpstream(ctx, upstream) + if valid == nil { + requeue = true + } else { + validatedUpstreams = append(validatedUpstreams, upstreamprovider.UpstreamGithubIdentityProviderI(valid)) + } + } + c.cache.SetGitHubIdentityProviders(validatedUpstreams) + if requeue { + return controllerlib.ErrSyntheticRequeue + } + return nil +} + +func (c *gitHubWatcherController) validateUpstream(ctx controllerlib.Context, upstream *v1alpha1.GitHubIdentityProvider) *upstreamgithub.ProviderConfig { + result := upstreamgithub.ProviderConfig{ + Name: upstream.Name, + } + + conditions := []*metav1.Condition{ + // TODO: once we firm up the proposal doc & merge, then firm up the CRD & merge, we can + // fill out these validations. + // c.validateHost(), + // c.validateTLS(), + // c.validateAllowedOrganizations(), + // c.validateOrganizationLoginPolicy(), + // c.validateClient(), + } + + c.updateStatus(ctx.Context, upstream, conditions) + + return &result +} + +func (c *gitHubWatcherController) updateStatus(ctx context.Context, upstream *v1alpha1.GitHubIdentityProvider, conditions []*metav1.Condition) { + log := c.log.WithValues("namespace", upstream.Namespace, "name", upstream.Name) + updated := upstream.DeepCopy() + + hadErrorCondition := conditionsutil.MergeIDPConditions(conditions, upstream.Generation, &updated.Status.Conditions, log) + + updated.Status.Phase = v1alpha1.GitHubPhaseReady + if hadErrorCondition { + updated.Status.Phase = v1alpha1.GitHubPhaseError + } + + if equality.Semantic.DeepEqual(upstream, updated) { + return + } + + _, err := c.client. + IDPV1alpha1(). + GitHubIdentityProviders(upstream.Namespace). + UpdateStatus(ctx, updated, metav1.UpdateOptions{}) + if err != nil { + log.Error(err, "failed to update status") + } +} diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go new file mode 100644 index 000000000..a95ee4abb --- /dev/null +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go @@ -0,0 +1,4 @@ +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package githubupstreamwatcher diff --git a/internal/federationdomain/dynamicupstreamprovider/dynamic_upstream_idp_provider.go b/internal/federationdomain/dynamicupstreamprovider/dynamic_upstream_idp_provider.go index bb92e6105..ea0cd50e8 100644 --- a/internal/federationdomain/dynamicupstreamprovider/dynamic_upstream_idp_provider.go +++ b/internal/federationdomain/dynamicupstreamprovider/dynamic_upstream_idp_provider.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package dynamicupstreamprovider @@ -17,12 +17,15 @@ type DynamicUpstreamIDPProvider interface { GetLDAPIdentityProviders() []upstreamprovider.UpstreamLDAPIdentityProviderI SetActiveDirectoryIdentityProviders(adIDPs []upstreamprovider.UpstreamLDAPIdentityProviderI) GetActiveDirectoryIdentityProviders() []upstreamprovider.UpstreamLDAPIdentityProviderI + SetGitHubIdentityProviders(gitHubIDPs []upstreamprovider.UpstreamGithubIdentityProviderI) + GetGitHubIdentityProviders() []upstreamprovider.UpstreamGithubIdentityProviderI } type dynamicUpstreamIDPProvider struct { oidcUpstreams []upstreamprovider.UpstreamOIDCIdentityProviderI ldapUpstreams []upstreamprovider.UpstreamLDAPIdentityProviderI activeDirectoryUpstreams []upstreamprovider.UpstreamLDAPIdentityProviderI + gitHubUpstreams []upstreamprovider.UpstreamGithubIdentityProviderI mutex sync.RWMutex } @@ -31,6 +34,7 @@ func NewDynamicUpstreamIDPProvider() DynamicUpstreamIDPProvider { oidcUpstreams: []upstreamprovider.UpstreamOIDCIdentityProviderI{}, ldapUpstreams: []upstreamprovider.UpstreamLDAPIdentityProviderI{}, activeDirectoryUpstreams: []upstreamprovider.UpstreamLDAPIdentityProviderI{}, + gitHubUpstreams: []upstreamprovider.UpstreamGithubIdentityProviderI{}, } } @@ -70,6 +74,18 @@ func (p *dynamicUpstreamIDPProvider) GetActiveDirectoryIdentityProviders() []ups return p.activeDirectoryUpstreams } +func (p *dynamicUpstreamIDPProvider) SetGitHubIdentityProviders(gitHubIDPs []upstreamprovider.UpstreamGithubIdentityProviderI) { + p.mutex.Lock() // acquire a write lock + defer p.mutex.Unlock() + p.gitHubUpstreams = gitHubIDPs +} + +func (p *dynamicUpstreamIDPProvider) GetGitHubIdentityProviders() []upstreamprovider.UpstreamGithubIdentityProviderI { + p.mutex.RLock() // acquire a read lock + defer p.mutex.RUnlock() + return p.gitHubUpstreams +} + type RetryableRevocationError struct { wrapped error } diff --git a/internal/federationdomain/upstreamprovider/upsteam_provider.go b/internal/federationdomain/upstreamprovider/upsteam_provider.go index 40162c52a..5a7c59fa4 100644 --- a/internal/federationdomain/upstreamprovider/upsteam_provider.go +++ b/internal/federationdomain/upstreamprovider/upsteam_provider.go @@ -125,3 +125,7 @@ type UpstreamLDAPIdentityProviderI interface { // PerformRefresh performs a refresh against the upstream LDAP identity provider PerformRefresh(ctx context.Context, storedRefreshAttributes RefreshAttributes, idpDisplayName string) (groups []string, err error) } + +type UpstreamGithubIdentityProviderI interface { + UpstreamIdentityProviderI +} diff --git a/internal/supervisor/server/server.go b/internal/supervisor/server/server.go index 0b17f0d6e..175852f2a 100644 --- a/internal/supervisor/server/server.go +++ b/internal/supervisor/server/server.go @@ -50,6 +50,7 @@ import ( "go.pinniped.dev/internal/controller/supervisorconfig" "go.pinniped.dev/internal/controller/supervisorconfig/activedirectoryupstreamwatcher" "go.pinniped.dev/internal/controller/supervisorconfig/generator" + "go.pinniped.dev/internal/controller/supervisorconfig/githubupstreamwatcher" "go.pinniped.dev/internal/controller/supervisorconfig/ldapupstreamwatcher" "go.pinniped.dev/internal/controller/supervisorconfig/oidcclientwatcher" "go.pinniped.dev/internal/controller/supervisorconfig/oidcupstreamwatcher" @@ -322,6 +323,18 @@ func prepareControllers( controllerlib.WithInformer, ), singletonWorker). + WithController( + githubupstreamwatcher.New( + dynamicUpstreamIDPProvider, + pinnipedClient, + pinnipedInformers.IDP().V1alpha1().GitHubIdentityProviders(), + secretInformer, + // TODO: need to swap this out for plog.New() + plog.Logr(), //nolint:staticcheck // old controller with lots of log statements + // plog.New(), + controllerlib.WithInformer, + ), + singletonWorker). WithController( apicerts.NewCertsManagerController( podInfo.Namespace, diff --git a/internal/upstreamgithub/upstreamgithub.go b/internal/upstreamgithub/upstreamgithub.go new file mode 100644 index 000000000..4bcddf568 --- /dev/null +++ b/internal/upstreamgithub/upstreamgithub.go @@ -0,0 +1,29 @@ +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package upstreamoidc implements an abstraction of upstream GitHub provider interactions. +package upstreamgithub + +import ( + "k8s.io/apimachinery/pkg/types" + + "go.pinniped.dev/internal/federationdomain/upstreamprovider" +) + +// ProviderConfig holds the active configuration of an upstream GitHub provider. +type ProviderConfig struct { + Name string + ResourceUID types.UID + UsernameClaim string + GroupsClaim string +} + +var _ upstreamprovider.UpstreamGithubIdentityProviderI = (*ProviderConfig)(nil) + +func (p *ProviderConfig) GetResourceUID() types.UID { + return p.ResourceUID +} + +func (p *ProviderConfig) GetName() string { + return p.Name +} diff --git a/internal/upstreamgithub/upstreamgithub_test.go b/internal/upstreamgithub/upstreamgithub_test.go new file mode 100644 index 000000000..37e13266f --- /dev/null +++ b/internal/upstreamgithub/upstreamgithub_test.go @@ -0,0 +1,4 @@ +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package upstreamgithub