refactor: rename "provider" to "federationdomain" when appropriate
Co-authored-by: Ryan Richard <richardry@vmware.com>
This commit is contained in:
parent
96098841dd
commit
5c0425fb71
@ -30,15 +30,15 @@ import (
|
||||
"go.pinniped.dev/internal/plog"
|
||||
)
|
||||
|
||||
// ProvidersSetter can be notified of all known valid providers with its SetIssuer function.
|
||||
// FederationDomainsSetter can be notified of all known valid providers with its SetIssuer function.
|
||||
// If there are no longer any valid issuers, then it can be called with no arguments.
|
||||
// Implementations of this type should be thread-safe to support calls from multiple goroutines.
|
||||
type ProvidersSetter interface {
|
||||
SetProviders(federationDomains ...*provider.FederationDomainIssuer)
|
||||
type FederationDomainsSetter interface {
|
||||
SetFederationDomains(federationDomains ...*provider.FederationDomainIssuer)
|
||||
}
|
||||
|
||||
type federationDomainWatcherController struct {
|
||||
providerSetter ProvidersSetter
|
||||
federationDomainsSetter FederationDomainsSetter
|
||||
clock clock.Clock
|
||||
client pinnipedclientset.Interface
|
||||
|
||||
@ -51,7 +51,7 @@ type federationDomainWatcherController struct {
|
||||
// NewFederationDomainWatcherController creates a controllerlib.Controller that watches
|
||||
// FederationDomain objects and notifies a callback object of the collection of provider configs.
|
||||
func NewFederationDomainWatcherController(
|
||||
providerSetter ProvidersSetter,
|
||||
federationDomainsSetter FederationDomainsSetter,
|
||||
clock clock.Clock,
|
||||
client pinnipedclientset.Interface,
|
||||
federationDomainInformer configinformers.FederationDomainInformer,
|
||||
@ -64,7 +64,7 @@ func NewFederationDomainWatcherController(
|
||||
controllerlib.Config{
|
||||
Name: "FederationDomainWatcherController",
|
||||
Syncer: &federationDomainWatcherController{
|
||||
providerSetter: providerSetter,
|
||||
federationDomainsSetter: federationDomainsSetter,
|
||||
clock: clock,
|
||||
client: client,
|
||||
federationDomainInformer: federationDomainInformer,
|
||||
@ -438,7 +438,7 @@ func (c *federationDomainWatcherController) Sync(ctx controllerlib.Context) erro
|
||||
federationDomainIssuers = append(federationDomainIssuers, federationDomainIssuer)
|
||||
}
|
||||
|
||||
c.providerSetter.SetProviders(federationDomainIssuers...)
|
||||
c.federationDomainsSetter.SetFederationDomains(federationDomainIssuers...)
|
||||
|
||||
return errors.NewAggregate(errs)
|
||||
}
|
||||
|
@ -89,13 +89,13 @@ func TestInformerFilters(t *testing.T) {
|
||||
}, spec.Parallel(), spec.Report(report.Terminal{}))
|
||||
}
|
||||
|
||||
type fakeProvidersSetter struct {
|
||||
SetProvidersWasCalled bool
|
||||
type fakeFederationDomainsSetter struct {
|
||||
SetFederationDomainsWasCalled bool
|
||||
FederationDomainsReceived []*provider.FederationDomainIssuer
|
||||
}
|
||||
|
||||
func (f *fakeProvidersSetter) SetProviders(federationDomains ...*provider.FederationDomainIssuer) {
|
||||
f.SetProvidersWasCalled = true
|
||||
func (f *fakeFederationDomainsSetter) SetFederationDomains(federationDomains ...*provider.FederationDomainIssuer) {
|
||||
f.SetFederationDomainsWasCalled = true
|
||||
f.FederationDomainsReceived = federationDomains
|
||||
}
|
||||
|
||||
@ -113,7 +113,7 @@ func TestSync(t *testing.T) {
|
||||
var cancelContextCancelFunc context.CancelFunc
|
||||
var syncContext *controllerlib.Context
|
||||
var frozenNow time.Time
|
||||
var providersSetter *fakeProvidersSetter
|
||||
var federationDomainsSetter *fakeFederationDomainsSetter
|
||||
var federationDomainGVR schema.GroupVersionResource
|
||||
|
||||
// Defer starting the informers until the last possible moment so that the
|
||||
@ -121,7 +121,7 @@ func TestSync(t *testing.T) {
|
||||
var startInformersAndController = func() {
|
||||
// Set this at the last second to allow for injection of server override.
|
||||
subject = NewFederationDomainWatcherController(
|
||||
providersSetter,
|
||||
federationDomainsSetter,
|
||||
clocktesting.NewFakeClock(frozenNow),
|
||||
pinnipedAPIClient,
|
||||
pinnipedInformers.Config().V1alpha1().FederationDomains(),
|
||||
@ -149,7 +149,7 @@ func TestSync(t *testing.T) {
|
||||
it.Before(func() {
|
||||
r = require.New(t)
|
||||
|
||||
providersSetter = &fakeProvidersSetter{}
|
||||
federationDomainsSetter = &fakeFederationDomainsSetter{}
|
||||
frozenNow = time.Date(2020, time.September, 23, 7, 42, 0, 0, time.Local)
|
||||
|
||||
cancelContext, cancelContextCancelFunc = context.WithCancel(context.Background())
|
||||
@ -191,24 +191,24 @@ func TestSync(t *testing.T) {
|
||||
r.NoError(pinnipedInformerClient.Tracker().Add(federationDomain2))
|
||||
})
|
||||
|
||||
it("calls the ProvidersSetter", func() {
|
||||
it("calls the FederationDomainsSetter", func() {
|
||||
startInformersAndController()
|
||||
err := controllerlib.TestSync(t, subject, *syncContext)
|
||||
r.NoError(err)
|
||||
|
||||
provider1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
fd1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
|
||||
provider2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
fd2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
|
||||
r.True(providersSetter.SetProvidersWasCalled)
|
||||
r.True(federationDomainsSetter.SetFederationDomainsWasCalled)
|
||||
r.ElementsMatch(
|
||||
[]*provider.FederationDomainIssuer{
|
||||
provider1,
|
||||
provider2,
|
||||
fd1,
|
||||
fd2,
|
||||
},
|
||||
providersSetter.FederationDomainsReceived,
|
||||
federationDomainsSetter.FederationDomainsReceived,
|
||||
)
|
||||
})
|
||||
|
||||
@ -292,24 +292,24 @@ func TestSync(t *testing.T) {
|
||||
r.ElementsMatch(expectedActions, pinnipedAPIClient.Actions())
|
||||
})
|
||||
|
||||
it("calls the ProvidersSetter with both FederationDomain's", func() {
|
||||
it("calls the FederationDomainsSetter with both FederationDomain's", func() {
|
||||
startInformersAndController()
|
||||
err := controllerlib.TestSync(t, subject, *syncContext)
|
||||
r.NoError(err)
|
||||
|
||||
provider1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
fd1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
|
||||
provider2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
fd2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
|
||||
r.True(providersSetter.SetProvidersWasCalled)
|
||||
r.True(federationDomainsSetter.SetFederationDomainsWasCalled)
|
||||
r.ElementsMatch(
|
||||
[]*provider.FederationDomainIssuer{
|
||||
provider1,
|
||||
provider2,
|
||||
fd1,
|
||||
fd2,
|
||||
},
|
||||
providersSetter.FederationDomainsReceived,
|
||||
federationDomainsSetter.FederationDomainsReceived,
|
||||
)
|
||||
})
|
||||
})
|
||||
@ -330,22 +330,22 @@ func TestSync(t *testing.T) {
|
||||
)
|
||||
})
|
||||
|
||||
it("sets the provider that it could actually update in the API", func() {
|
||||
it("sets the FederationDomain that it could actually update in the API", func() {
|
||||
startInformersAndController()
|
||||
err := controllerlib.TestSync(t, subject, *syncContext)
|
||||
r.EqualError(err, "could not update status: some update error")
|
||||
|
||||
provider1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
fd1, err := provider.NewFederationDomainIssuer(federationDomain1.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
|
||||
provider2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
fd2, err := provider.NewFederationDomainIssuer(federationDomain2.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
|
||||
r.True(providersSetter.SetProvidersWasCalled)
|
||||
r.Len(providersSetter.FederationDomainsReceived, 1)
|
||||
r.True(federationDomainsSetter.SetFederationDomainsWasCalled)
|
||||
r.Len(federationDomainsSetter.FederationDomainsReceived, 1)
|
||||
r.True(
|
||||
reflect.DeepEqual(providersSetter.FederationDomainsReceived[0], provider1) ||
|
||||
reflect.DeepEqual(providersSetter.FederationDomainsReceived[0], provider2),
|
||||
reflect.DeepEqual(federationDomainsSetter.FederationDomainsReceived[0], fd1) ||
|
||||
reflect.DeepEqual(federationDomainsSetter.FederationDomainsReceived[0], fd2),
|
||||
)
|
||||
})
|
||||
|
||||
@ -549,20 +549,20 @@ func TestSync(t *testing.T) {
|
||||
r.NoError(pinnipedInformerClient.Tracker().Add(invalidFederationDomain))
|
||||
})
|
||||
|
||||
it("calls the ProvidersSetter with the valid provider", func() {
|
||||
it("calls the FederationDomainsSetter with the valid FederationDomain", func() {
|
||||
startInformersAndController()
|
||||
err := controllerlib.TestSync(t, subject, *syncContext)
|
||||
r.NoError(err)
|
||||
|
||||
validProvider, err := provider.NewFederationDomainIssuer(validFederationDomain.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
validFederationDomain, err := provider.NewFederationDomainIssuer(validFederationDomain.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
|
||||
r.True(providersSetter.SetProvidersWasCalled)
|
||||
r.True(federationDomainsSetter.SetFederationDomainsWasCalled)
|
||||
r.Equal(
|
||||
[]*provider.FederationDomainIssuer{
|
||||
validProvider,
|
||||
validFederationDomain,
|
||||
},
|
||||
providersSetter.FederationDomainsReceived,
|
||||
federationDomainsSetter.FederationDomainsReceived,
|
||||
)
|
||||
})
|
||||
|
||||
@ -623,20 +623,20 @@ func TestSync(t *testing.T) {
|
||||
)
|
||||
})
|
||||
|
||||
it("sets the provider that it could actually update in the API", func() {
|
||||
it("sets the FederationDomain that it could actually update in the API", func() {
|
||||
startInformersAndController()
|
||||
err := controllerlib.TestSync(t, subject, *syncContext)
|
||||
r.EqualError(err, "could not update status: some update error")
|
||||
|
||||
validProvider, err := provider.NewFederationDomainIssuer(validFederationDomain.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
validFederationDomain, err := provider.NewFederationDomainIssuer(validFederationDomain.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
|
||||
r.True(providersSetter.SetProvidersWasCalled)
|
||||
r.True(federationDomainsSetter.SetFederationDomainsWasCalled)
|
||||
r.Equal(
|
||||
[]*provider.FederationDomainIssuer{
|
||||
validProvider,
|
||||
validFederationDomain,
|
||||
},
|
||||
providersSetter.FederationDomainsReceived,
|
||||
federationDomainsSetter.FederationDomainsReceived,
|
||||
)
|
||||
})
|
||||
|
||||
@ -713,20 +713,20 @@ func TestSync(t *testing.T) {
|
||||
r.NoError(pinnipedInformerClient.Tracker().Add(federationDomain))
|
||||
})
|
||||
|
||||
it("calls the ProvidersSetter with the non-duplicate", func() {
|
||||
it("calls the FederationDomainsSetter with the non-duplicate", func() {
|
||||
startInformersAndController()
|
||||
err := controllerlib.TestSync(t, subject, *syncContext)
|
||||
r.NoError(err)
|
||||
|
||||
nonDuplicateProvider, err := provider.NewFederationDomainIssuer(federationDomain.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
nonDuplicateFederationDomain, err := provider.NewFederationDomainIssuer(federationDomain.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
|
||||
r.True(providersSetter.SetProvidersWasCalled)
|
||||
r.True(federationDomainsSetter.SetFederationDomainsWasCalled)
|
||||
r.Equal(
|
||||
[]*provider.FederationDomainIssuer{
|
||||
nonDuplicateProvider,
|
||||
nonDuplicateFederationDomain,
|
||||
},
|
||||
providersSetter.FederationDomainsReceived,
|
||||
federationDomainsSetter.FederationDomainsReceived,
|
||||
)
|
||||
})
|
||||
|
||||
@ -840,7 +840,7 @@ func TestSync(t *testing.T) {
|
||||
|
||||
it.Before(func() {
|
||||
federationDomainSameIssuerAddress1 = &v1alpha1.FederationDomain{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "provider1", Namespace: namespace},
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "fd1", Namespace: namespace},
|
||||
Spec: v1alpha1.FederationDomainSpec{
|
||||
Issuer: "https://iSSueR-duPlicAte-adDress.cOm/path1",
|
||||
TLS: &v1alpha1.FederationDomainTLSSpec{SecretName: "secret1"},
|
||||
@ -849,7 +849,7 @@ func TestSync(t *testing.T) {
|
||||
r.NoError(pinnipedAPIClient.Tracker().Add(federationDomainSameIssuerAddress1))
|
||||
r.NoError(pinnipedInformerClient.Tracker().Add(federationDomainSameIssuerAddress1))
|
||||
federationDomainSameIssuerAddress2 = &v1alpha1.FederationDomain{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "provider2", Namespace: namespace},
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "fd2", Namespace: namespace},
|
||||
Spec: v1alpha1.FederationDomainSpec{
|
||||
// Validation treats these as the same DNS hostname even though they have different port numbers,
|
||||
// because SNI information on the incoming requests is not going to include port numbers.
|
||||
@ -861,7 +861,7 @@ func TestSync(t *testing.T) {
|
||||
r.NoError(pinnipedInformerClient.Tracker().Add(federationDomainSameIssuerAddress2))
|
||||
|
||||
federationDomainDifferentIssuerAddress = &v1alpha1.FederationDomain{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "differentIssuerAddressProvider", Namespace: namespace},
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "differentIssuerAddressFederationDomain", Namespace: namespace},
|
||||
Spec: v1alpha1.FederationDomainSpec{
|
||||
Issuer: "https://issuer-not-duplicate.com",
|
||||
TLS: &v1alpha1.FederationDomainTLSSpec{SecretName: "secret1"},
|
||||
@ -876,7 +876,7 @@ func TestSync(t *testing.T) {
|
||||
_, err := url.Parse(invalidIssuerURL) //nolint:staticcheck // Yes, this URL is intentionally invalid.
|
||||
r.Error(err)
|
||||
federationDomainWithInvalidIssuerURL = &v1alpha1.FederationDomain{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "invalidIssuerURLProvider", Namespace: namespace},
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "invalidIssuerURLFederationDomain", Namespace: namespace},
|
||||
Spec: v1alpha1.FederationDomainSpec{
|
||||
Issuer: invalidIssuerURL,
|
||||
TLS: &v1alpha1.FederationDomainTLSSpec{SecretName: "secret1"},
|
||||
@ -886,20 +886,20 @@ func TestSync(t *testing.T) {
|
||||
r.NoError(pinnipedInformerClient.Tracker().Add(federationDomainWithInvalidIssuerURL))
|
||||
})
|
||||
|
||||
it("calls the ProvidersSetter with the non-duplicate", func() {
|
||||
it("calls the FederationDomainsSetter with the non-duplicate", func() {
|
||||
startInformersAndController()
|
||||
err := controllerlib.TestSync(t, subject, *syncContext)
|
||||
r.NoError(err)
|
||||
|
||||
nonDuplicateProvider, err := provider.NewFederationDomainIssuer(federationDomainDifferentIssuerAddress.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
nonDuplicateFederationDomain, err := provider.NewFederationDomainIssuer(federationDomainDifferentIssuerAddress.Spec.Issuer, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
|
||||
r.True(providersSetter.SetProvidersWasCalled)
|
||||
r.True(federationDomainsSetter.SetFederationDomainsWasCalled)
|
||||
r.Equal(
|
||||
[]*provider.FederationDomainIssuer{
|
||||
nonDuplicateProvider,
|
||||
nonDuplicateFederationDomain,
|
||||
},
|
||||
providersSetter.FederationDomainsReceived,
|
||||
federationDomainsSetter.FederationDomainsReceived,
|
||||
)
|
||||
})
|
||||
|
||||
@ -1029,8 +1029,8 @@ func TestSync(t *testing.T) {
|
||||
err := controllerlib.TestSync(t, subject, *syncContext)
|
||||
r.NoError(err)
|
||||
r.Empty(pinnipedAPIClient.Actions())
|
||||
r.True(providersSetter.SetProvidersWasCalled)
|
||||
r.Empty(providersSetter.FederationDomainsReceived)
|
||||
r.True(federationDomainsSetter.SetFederationDomainsWasCalled)
|
||||
r.Empty(federationDomainsSetter.FederationDomainsReceived)
|
||||
})
|
||||
})
|
||||
}, spec.Parallel(), spec.Report(report.Terminal{}))
|
||||
|
@ -69,7 +69,7 @@ func NewManager(
|
||||
}
|
||||
}
|
||||
|
||||
// SetProviders adds or updates all the given providerHandlers using each provider's issuer string
|
||||
// SetFederationDomains adds or updates all the given providerHandlers using each provider's issuer string
|
||||
// as the name of the provider to decide if it is an add or update operation.
|
||||
//
|
||||
// It also removes any providerHandlers that were previously added but were not passed in to
|
||||
@ -77,7 +77,7 @@ func NewManager(
|
||||
//
|
||||
// This method assumes that all of the FederationDomainIssuer arguments have already been validated
|
||||
// by someone else before they are passed to this method.
|
||||
func (m *Manager) SetProviders(federationDomains ...*provider.FederationDomainIssuer) {
|
||||
func (m *Manager) SetFederationDomains(federationDomains ...*provider.FederationDomainIssuer) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
|
||||
|
@ -277,7 +277,7 @@ func TestManager(t *testing.T) {
|
||||
subject = NewManager(nextHandler, dynamicJWKSProvider, idpLister, &cache, secretsClient, oidcClientsClient)
|
||||
})
|
||||
|
||||
when("given no providers via SetProviders()", func() {
|
||||
when("given no providers via SetFederationDomains()", func() {
|
||||
it("sends all requests to the nextHandler", func() {
|
||||
r.False(fallbackHandlerWasCalled)
|
||||
subject.ServeHTTP(httptest.NewRecorder(), newGetRequest("/anything"))
|
||||
@ -375,13 +375,13 @@ func TestManager(t *testing.T) {
|
||||
requireTokenRequestToBeHandled(issuer2DifferentCaseHostname, downstreamAuthCode4, issuer2JWKS, issuer2)
|
||||
}
|
||||
|
||||
when("given some valid providers via SetProviders()", func() {
|
||||
when("given some valid providers via SetFederationDomains()", func() {
|
||||
it.Before(func() {
|
||||
p1, err := provider.NewFederationDomainIssuer(issuer1, []*provider.FederationDomainIdentityProvider{})
|
||||
fd1, err := provider.NewFederationDomainIssuer(issuer1, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
p2, err := provider.NewFederationDomainIssuer(issuer2, []*provider.FederationDomainIdentityProvider{})
|
||||
fd2, err := provider.NewFederationDomainIssuer(issuer2, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
subject.SetProviders(p1, p2)
|
||||
subject.SetFederationDomains(fd1, fd2)
|
||||
|
||||
jwksMap := map[string]*jose.JSONWebKeySet{
|
||||
issuer1: {Keys: []jose.JSONWebKey{*newTestJWK(issuer1KeyID)}},
|
||||
@ -418,13 +418,13 @@ func TestManager(t *testing.T) {
|
||||
})
|
||||
})
|
||||
|
||||
when("given the same valid providers as arguments to SetProviders() in reverse order", func() {
|
||||
when("given the same valid providers as arguments to SetFederationDomains() in reverse order", func() {
|
||||
it.Before(func() {
|
||||
p1, err := provider.NewFederationDomainIssuer(issuer1, []*provider.FederationDomainIdentityProvider{})
|
||||
fd1, err := provider.NewFederationDomainIssuer(issuer1, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
p2, err := provider.NewFederationDomainIssuer(issuer2, []*provider.FederationDomainIdentityProvider{})
|
||||
fd2, err := provider.NewFederationDomainIssuer(issuer2, []*provider.FederationDomainIdentityProvider{})
|
||||
r.NoError(err)
|
||||
subject.SetProviders(p2, p1)
|
||||
subject.SetFederationDomains(fd2, fd1)
|
||||
|
||||
jwksMap := map[string]*jose.JSONWebKeySet{
|
||||
issuer1: {Keys: []jose.JSONWebKey{*newTestJWK(issuer1KeyID)}},
|
||||
|
Loading…
Reference in New Issue
Block a user