Stub in TestUpstreamGithubIdentityProvider for unit tests

internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go

@@ -12,6 +12,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	errorsutil "k8s.io/apimachinery/pkg/util/errors"
 	corev1informers "k8s.io/client-go/informers/core/v1"
 
 	"go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
@@ -92,12 +93,15 @@ func (c *gitHubWatcherController) Sync(ctx controllerlib.Context) error {
 		return fmt.Errorf("failed to list GitHubIdentityProviders: %w", err)
 	}
 
+	var errs []error
+
 	requeue := false
 	validatedUpstreams := make([]upstreamprovider.UpstreamGithubIdentityProviderI, 0, len(actualUpstreams))
 	for _, upstream := range actualUpstreams {
-		valid := c.validateUpstream(ctx, upstream)
+		valid, err := c.validateUpstream(ctx, upstream)
 		if valid == nil {
 			requeue = true
+			errs = append(errs, err)
 		} else {
 			validatedUpstreams = append(validatedUpstreams, upstreamprovider.UpstreamGithubIdentityProviderI(valid))
 		}
@@ -106,10 +110,16 @@ func (c *gitHubWatcherController) Sync(ctx controllerlib.Context) error {
 	if requeue {
 		return controllerlib.ErrSyntheticRequeue
 	}
-	return nil
+
+	// Sync loop errors:
+	// - Should not be configuration errors. Config errors a user must correct belong on the .Status
+	//   object. The controller simply must wait for a user to correct before running again.
+	// - Other errors, such as networking errors, etc. are the types of errors that should return here
+	//   and signal the controller to retry the sync loop. These may be corrected by machines.
+	return errorsutil.NewAggregate(errs)
 }
 
-func (c *gitHubWatcherController) validateUpstream(ctx controllerlib.Context, upstream *v1alpha1.GitHubIdentityProvider) *upstreamgithub.ProviderConfig {
+func (c *gitHubWatcherController) validateUpstream(ctx controllerlib.Context, upstream *v1alpha1.GitHubIdentityProvider) (*upstreamgithub.ProviderConfig, error) {
 	result := upstreamgithub.ProviderConfig{
 		Name: upstream.Name,
 	}
@@ -127,9 +137,8 @@ func (c *gitHubWatcherController) validateUpstream(ctx controllerlib.Context, up
 		// c.validateClient(),
 	}
 
-	c.updateStatus(ctx.Context, upstream, conditions)
-
-	return &result
+	err := c.updateStatus(ctx.Context, upstream, conditions)
+	return &result, err
 }
 
 func (c *gitHubWatcherController) updateStatus(

internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go

@@ -9,22 +9,29 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	k8sinformers "k8s.io/client-go/informers"
 	kubernetesfake "k8s.io/client-go/kubernetes/fake"
+	"k8s.io/utils/ptr"
 
 	"go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1"
 	pinnipedfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake"
 	pinnipedinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/federationdomain/dynamicupstreamprovider"
+	"go.pinniped.dev/internal/federationdomain/upstreamprovider"
 	"go.pinniped.dev/internal/plog"
-	"go.pinniped.dev/internal/testutil/oidctestutil"
+	"go.pinniped.dev/internal/testutil/githubtestutil"
+	"go.pinniped.dev/internal/upstreamgithub"
 )
 
 func TestController(t *testing.T) {
 	t.Parallel()
 
+	testNamespace := "foo"
+	testName := "bar"
+
 	tests := []struct {
 		name                    string
 		githubIdentityProviders []runtime.Object
@@ -32,11 +39,41 @@ func TestController(t *testing.T) {
 		configClient            func(*pinnipedfake.Clientset)
 		wantErr                 string
 		wantLogs                []string
-		wantResultingCache      []*oidctestutil.TestUpstreamOIDCIdentityProvider
+		wantResultingCache      []*githubtestutil.TestUpstreamGithubIdentityProvider
 		wantResultingUpstreams  []v1alpha1.GitHubIdentityProvider
 	}{
 		{
-			name: "The controller runs.... and should be further tested.",
+			name: "no upstreams",
+		}, {
+			name:         "found github idp is cached",
+			inputSecrets: []runtime.Object{},
+			githubIdentityProviders: []runtime.Object{&v1alpha1.GitHubIdentityProvider{
+				ObjectMeta: v1.ObjectMeta{Namespace: testNamespace, Name: testName},
+				Spec: v1alpha1.GitHubIdentityProviderSpec{
+					GitHubAPI: v1alpha1.GitHubAPIConfig{
+						Host: ptr.To("127.0.0.1"),
+						TLS: &v1alpha1.TLSSpec{
+							CertificateAuthorityData: "irrelevant",
+						},
+					},
+					Claims: v1alpha1.GitHubClaims{
+						Username: ptr.To(v1alpha1.GitHubUsernameID),
+						Groups:   ptr.To(v1alpha1.GitHubUseTeamNameForGroupName),
+					},
+					AllowAuthentication: v1alpha1.GitHubAllowAuthenticationSpec{
+						Organizations: v1alpha1.GitHubOrganizationsSpec{
+							Policy:  ptr.To(v1alpha1.GitHubAllowedAuthOrganizationsPolicyAllGitHubUsers),
+							Allowed: []string{"foo", "bar"},
+						},
+					},
+					Client: v1alpha1.GitHubClientSpec{
+						SecretName: "some-secret",
+					},
+				},
+			}},
+			wantResultingCache: []*githubtestutil.TestUpstreamGithubIdentityProvider{{
+				Name: "test-github-idp-to-flesh-out",
+			}},
 		},
 	}
 
@@ -45,7 +82,6 @@ func TestController(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			dynamicUpstreamProvider := dynamicupstreamprovider.NewDynamicUpstreamIDPProvider()
 			pinnipedAPIClient := pinnipedfake.NewSimpleClientset(tt.githubIdentityProviders...)
 			fakePinnipedClientForInformers := pinnipedfake.NewSimpleClientset(tt.githubIdentityProviders...)
 			pinnipedInformers := pinnipedinformers.NewSharedInformerFactory(fakePinnipedClientForInformers, 0)
@@ -53,11 +89,16 @@ func TestController(t *testing.T) {
 			fakeKubeClient := kubernetesfake.NewSimpleClientset(tt.inputSecrets...)
 			kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(fakeKubeClient, 0)
 
+			cache := dynamicupstreamprovider.NewDynamicUpstreamIDPProvider()
+			cache.SetGitHubIdentityProviders([]upstreamprovider.UpstreamGithubIdentityProviderI{
+				&upstreamgithub.ProviderConfig{Name: "initial-entry-to-remove"},
+			})
+
 			var log bytes.Buffer
 			logger := plog.TestLogger(t, &log)
 
 			controller := New(
-				dynamicUpstreamProvider,
+				cache,
 				pinnipedAPIClient,
 				pinnipedInformers.IDP().V1alpha1().GitHubIdentityProviders(),
 				kubeInformers.Core().V1().Secrets(),
@@ -79,6 +120,9 @@ func TestController(t *testing.T) {
 			} else {
 				require.NoError(t, err)
 			}
+
+			actualIDPList := cache.GetGitHubIdentityProviders()
+			require.Equal(t, len(tt.wantResultingCache), len(actualIDPList))
 		})
 	}
 }

internal/testutil/githubtestutil/testgithubprovider.go

@@ -0,0 +1,8 @@
+// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package githubtestutil
+
+type TestUpstreamGithubIdentityProvider struct {
+	Name string
+}