Skip to content

Commit

Permalink
feat: make oidc discovery url configurable
Browse files Browse the repository at this point in the history
Signed-off-by: Calum Murray <[email protected]>
  • Loading branch information
Cali0707 authored and creydr committed Sep 17, 2024
1 parent e79f3b6 commit 3f2dfa6
Show file tree
Hide file tree
Showing 13 changed files with 61 additions and 36 deletions.
4 changes: 2 additions & 2 deletions cmd/broker/filter/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func main() {
}
handler.EventTypeCreator = autoCreate
}

handler.TokenVerifier = auth.NewVerifier(ctx, eventpolicyinformer.Get(ctx).Lister(), featureFlags)
})
featureStore.WatchConfigs(configMapWatcher)

Expand All @@ -154,7 +154,7 @@ func main() {
oidcTokenProvider := auth.NewOIDCTokenProvider(ctx)
// We are running both the receiver (takes messages in from the Broker) and the dispatcher (send
// the messages to the triggers' subscribers) in this binary.
authVerifier := auth.NewVerifier(ctx, eventpolicyinformer.Get(ctx).Lister())
authVerifier := auth.NewVerifier(ctx, eventpolicyinformer.Get(ctx).Lister(), featureStore.Load())
trustBundleConfigMapInformer := configmapinformer.Get(ctx, eventingtls.TrustBundleLabelSelector).Lister().ConfigMaps(system.Namespace())
handler, err = filter.NewHandler(logger, authVerifier, oidcTokenProvider, triggerinformer.Get(ctx), brokerinformer.Get(ctx), subscriptioninformer.Get(ctx), reporter, trustBundleConfigMapInformer, ctxFunc)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion cmd/broker/ingress/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ func main() {
}
handler.EvenTypeHandler = autoCreate
}
handler.TokenVerifier = auth.NewVerifier(ctx, eventpolicyinformer.Get(ctx).Lister(), featureFlags)
})
featureStore.WatchConfigs(configMapWatcher)

Expand All @@ -168,7 +169,7 @@ func main() {
reporter := ingress.NewStatsReporter(env.ContainerName, kmeta.ChildName(env.PodName, uuid.New().String()))

oidcTokenProvider := auth.NewOIDCTokenProvider(ctx)
authVerifier := auth.NewVerifier(ctx, eventpolicyinformer.Get(ctx).Lister())
authVerifier := auth.NewVerifier(ctx, eventpolicyinformer.Get(ctx).Lister(), featureStore.Load())
trustBundleConfigMapInformer := configmapinformer.Get(ctx, eventingtls.TrustBundleLabelSelector).Lister().ConfigMaps(system.Namespace())
handler, err = ingress.NewHandler(logger, reporter, broker.TTLDefaulter(logger, int32(env.MaxTTL)), brokerInformer, authVerifier, oidcTokenProvider, trustBundleConfigMapInformer, ctxFunc)
if err != nil {
Expand Down
7 changes: 6 additions & 1 deletion cmd/jobsink/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,13 @@ func main() {

logger.Info("Starting the JobSink Ingress")

var h *Handler

featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store"), func(name string, value interface{}) {
logger.Info("Updated", zap.String("name", name), zap.Any("value", value))
if flags, ok := value.(feature.Flags); ok && h != nil {
h.authVerifier = auth.NewVerifier(ctx, eventpolicyinformer.Get(ctx).Lister(), flags)
}
})
featureStore.WatchConfigs(configMapWatcher)

Expand All @@ -118,7 +123,7 @@ func main() {
k8s: kubeclient.Get(ctx),
lister: jobsink.Get(ctx).Lister(),
withContext: ctxFunc,
authVerifier: auth.NewVerifier(ctx, eventpolicyinformer.Get(ctx).Lister()),
authVerifier: auth.NewVerifier(ctx, eventpolicyinformer.Get(ctx).Lister(), featureStore.Load()),
}

tlsConfig, err := getServerTLSConfig(ctx)
Expand Down
19 changes: 18 additions & 1 deletion pkg/apis/feature/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ const (
// This configuration is applied when there is no EventPolicy with a "to" referencing a given
// resource.
AuthorizationAllowSameNamespace Flag = "Allow-Same-Namespace"

// DefaultOIDCDiscoveryURL is the default OIDC Discovery URL used in most Kubernetes clusters.
DefaultOIDCDiscoveryBaseURL Flag = "https://kubernetes.default.svc"
)

// Flags is a map containing all the enabled/disabled flags for the experimental features.
Expand All @@ -81,6 +84,7 @@ func newDefaults() Flags {
EvenTypeAutoCreate: Disabled,
NewAPIServerFilters: Disabled,
AuthorizationDefaultMode: AuthorizationAllowSameNamespace,
OIDCDiscoveryBaseURL: DefaultOIDCDiscoveryBaseURL,
}
}

Expand Down Expand Up @@ -134,6 +138,19 @@ func (e Flags) IsAuthorizationDefaultModeSameNamespace() bool {
return e != nil && e[AuthorizationDefaultMode] == AuthorizationAllowSameNamespace
}

func (e Flags) OIDCDiscoveryBaseURL() string {
if e == nil {
return string(DefaultOIDCDiscoveryBaseURL)
}

discoveryUrl, ok := e[OIDCDiscoveryBaseURL]
if !ok {
return string(DefaultOIDCDiscoveryBaseURL)
}

return string(discoveryUrl)
}

func (e Flags) String() string {
return fmt.Sprintf("%+v", map[string]Flag(e))
}
Expand Down Expand Up @@ -183,7 +200,7 @@ func NewFlagsConfigFromMap(data map[string]string) (Flags, error) {
flags[sanitizedKey] = AuthorizationDenyAll
} else if sanitizedKey == AuthorizationDefaultMode && strings.EqualFold(v, string(AuthorizationAllowSameNamespace)) {
flags[sanitizedKey] = AuthorizationAllowSameNamespace
} else if strings.Contains(k, NodeSelectorLabel) {
} else if strings.Contains(k, NodeSelectorLabel) || sanitizedKey == OIDCDiscoveryBaseURL {
flags[sanitizedKey] = Flag(v)
} else {
flags[k] = Flag(v)
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/feature/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ func TestGetFlags(t *testing.T) {
nodeSelector := flags.NodeSelector()
expectedNodeSelector := map[string]string{"testkey": "testvalue", "testkey1": "testvalue1", "testkey2": "testvalue2"}
require.Equal(t, expectedNodeSelector, nodeSelector)

require.Equal(t, flags.OIDCDiscoveryBaseURL(), "https://oidc.eks.eu-west-1.amazonaws.com/id/1")
}

func TestShouldNotOverrideDefaults(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/feature/flag_names.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,5 @@ const (
CrossNamespaceEventLinks = "cross-namespace-event-links"
NewAPIServerFilters = "new-apiserversource-filters"
AuthorizationDefaultMode = "default-authorization-mode"
OIDCDiscoveryBaseURL = "oidc-discovery-base-url"
)
1 change: 1 addition & 0 deletions pkg/apis/feature/testdata/config-features.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ data:
apiserversources-nodeselector-testkey: testvalue
apiserversources-nodeselector-testkey1: testvalue1
apiserversources-nodeselector-testkey2: testvalue2
oidc-discovery-base-url: "https://oidc.eks.eu-west-1.amazonaws.com/id/1"
20 changes: 8 additions & 12 deletions pkg/auth/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ import (
"knative.dev/pkg/logging"
)

const (
kubernetesOIDCDiscoveryBaseURL = "https://kubernetes.default.svc"
)

type Verifier struct {
logger *zap.SugaredLogger
restConfig *rest.Config
Expand All @@ -61,14 +57,14 @@ type IDToken struct {
AccessTokenHash string
}

func NewVerifier(ctx context.Context, eventPolicyLister listerseventingv1alpha1.EventPolicyLister) *Verifier {
func NewVerifier(ctx context.Context, eventPolicyLister listerseventingv1alpha1.EventPolicyLister, features feature.Flags) *Verifier {
tokenHandler := &Verifier{
logger: logging.FromContext(ctx).With("component", "oidc-token-handler"),
restConfig: injection.GetConfig(ctx),
eventPolicyLister: eventPolicyLister,
}

if err := tokenHandler.initOIDCProvider(ctx); err != nil {
if err := tokenHandler.initOIDCProvider(ctx, features); err != nil {
tokenHandler.logger.Error(fmt.Sprintf("could not initialize provider. You can ignore this message, when the %s feature is disabled", feature.OIDCAuthentication), zap.Error(err))
}

Expand Down Expand Up @@ -219,13 +215,13 @@ func (v *Verifier) verifyJWT(ctx context.Context, jwt, audience string) (*IDToke
}, nil
}

func (v *Verifier) initOIDCProvider(ctx context.Context) error {
discovery, err := v.getKubernetesOIDCDiscovery()
func (v *Verifier) initOIDCProvider(ctx context.Context, features feature.Flags) error {
discovery, err := v.getKubernetesOIDCDiscovery(features)
if err != nil {
return fmt.Errorf("could not load Kubernetes OIDC discovery information: %w", err)
}

if discovery.Issuer != kubernetesOIDCDiscoveryBaseURL {
if discovery.Issuer != features.OIDCDiscoveryBaseURL() {
// in case we have another issuer as the api server:
ctx = oidc.InsecureIssuerURLContext(ctx, discovery.Issuer)
}
Expand All @@ -237,7 +233,7 @@ func (v *Verifier) initOIDCProvider(ctx context.Context) error {
ctx = oidc.ClientContext(ctx, httpClient)

// get OIDC provider
v.provider, err = oidc.NewProvider(ctx, kubernetesOIDCDiscoveryBaseURL)
v.provider, err = oidc.NewProvider(ctx, features.OIDCDiscoveryBaseURL())
if err != nil {
return fmt.Errorf("could not get OIDC provider: %w", err)
}
Expand All @@ -256,13 +252,13 @@ func (v *Verifier) getHTTPClientForKubeAPIServer() (*http.Client, error) {
return client, nil
}

func (v *Verifier) getKubernetesOIDCDiscovery() (*openIDMetadata, error) {
func (v *Verifier) getKubernetesOIDCDiscovery(features feature.Flags) (*openIDMetadata, error) {
client, err := v.getHTTPClientForKubeAPIServer()
if err != nil {
return nil, fmt.Errorf("could not get HTTP client for API server: %w", err)
}

resp, err := client.Get(kubernetesOIDCDiscoveryBaseURL + "/.well-known/openid-configuration")
resp, err := client.Get(features.OIDCDiscoveryBaseURL() + "/.well-known/openid-configuration")
if err != nil {
return nil, fmt.Errorf("could not get response: %w", err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/broker/filter/filter_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ type Handler struct {
logger *zap.Logger
withContext func(ctx context.Context) context.Context
filtersMap *subscriptionsapi.FiltersMap
tokenVerifier *auth.Verifier
TokenVerifier *auth.Verifier
EventTypeCreator *eventtype.EventTypeAutoHandler
}

Expand Down Expand Up @@ -153,7 +153,7 @@ func NewHandler(logger *zap.Logger, tokenVerifier *auth.Verifier, oidcTokenProvi
brokerLister: brokerInformer.Lister(),
subscriptionLister: subscriptionInformer.Lister(),
logger: logger,
tokenVerifier: tokenVerifier,
TokenVerifier: tokenVerifier,
withContext: wc,
filtersMap: fm,
}, nil
Expand Down Expand Up @@ -225,7 +225,7 @@ func (h *Handler) ServeHTTP(writer http.ResponseWriter, request *http.Request) {
}

subscriptionFullIdentity := fmt.Sprintf("system:serviceaccount:%s:%s", subscription.Namespace, *subscription.Status.Auth.ServiceAccountName)
err = h.tokenVerifier.VerifyRequestFromSubject(ctx, features, &audience, subscriptionFullIdentity, request, writer)
err = h.TokenVerifier.VerifyRequestFromSubject(ctx, features, &audience, subscriptionFullIdentity, request, writer)
if err != nil {
h.logger.Warn("Error when validating the JWT token in the request", zap.Error(err))
return
Expand Down
4 changes: 2 additions & 2 deletions pkg/broker/filter/filter_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ func TestReceiver(t *testing.T) {

logger := zaptest.NewLogger(t, zaptest.WrapOptions(zap.AddCaller()))
oidcTokenProvider := auth.NewOIDCTokenProvider(ctx)
authVerifier := auth.NewVerifier(ctx, eventpolicyinformerfake.Get(ctx).Lister())
authVerifier := auth.NewVerifier(ctx, eventpolicyinformerfake.Get(ctx).Lister(), feature.FromContext(ctx))

for _, trig := range tc.triggers {
// Replace the SubscriberURI to point at our fake server.
Expand Down Expand Up @@ -653,7 +653,7 @@ func TestReceiver_WithSubscriptionsAPI(t *testing.T) {

logger := zaptest.NewLogger(t, zaptest.WrapOptions(zap.AddCaller()))
oidcTokenProvider := auth.NewOIDCTokenProvider(ctx)
authVerifier := auth.NewVerifier(ctx, eventpolicyinformerfake.Get(ctx).Lister())
authVerifier := auth.NewVerifier(ctx, eventpolicyinformerfake.Get(ctx).Lister(), feature.FromContext(ctx))

// Replace the SubscriberURI to point at our fake server.
for _, trig := range tc.triggers {
Expand Down
6 changes: 3 additions & 3 deletions pkg/broker/ingress/ingress_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ type Handler struct {

eventDispatcher *kncloudevents.Dispatcher

tokenVerifier *auth.Verifier
TokenVerifier *auth.Verifier

withContext func(ctx context.Context) context.Context
}
Expand Down Expand Up @@ -128,7 +128,7 @@ func NewHandler(logger *zap.Logger, reporter StatsReporter, defaulter client.Eve
Logger: logger,
BrokerLister: brokerInformer.Lister(),
eventDispatcher: kncloudevents.NewDispatcher(clientConfig, oidcTokenProvider),
tokenVerifier: tokenVerifier,
TokenVerifier: tokenVerifier,
withContext: withContext,
}, nil
}
Expand Down Expand Up @@ -237,7 +237,7 @@ func (h *Handler) ServeHTTP(writer http.ResponseWriter, request *http.Request) {
if broker.Status.Address != nil {
audience = broker.Status.Address.Audience
}
err = h.tokenVerifier.VerifyRequest(ctx, features, audience, brokerNamespace, broker.Status.Policies, request, writer)
err = h.TokenVerifier.VerifyRequest(ctx, features, audience, brokerNamespace, broker.Status.Policies, request, writer)
if err != nil {
h.Logger.Warn("Failed to verify AuthN and AuthZ.", zap.Error(err))
return
Expand Down
3 changes: 2 additions & 1 deletion pkg/broker/ingress/ingress_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (

"knative.dev/eventing/pkg/apis/eventing"
eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1"
"knative.dev/eventing/pkg/apis/feature"
"knative.dev/eventing/pkg/auth"
"knative.dev/eventing/pkg/broker"

Expand Down Expand Up @@ -291,7 +292,7 @@ func TestHandler_ServeHTTP(t *testing.T) {
}

tokenProvider := auth.NewOIDCTokenProvider(ctx)
authVerifier := auth.NewVerifier(ctx, eventpolicyinformerfake.Get(ctx).Lister())
authVerifier := auth.NewVerifier(ctx, eventpolicyinformerfake.Get(ctx).Lister(), feature.FromContextOrDefaults(ctx))

h, err := NewHandler(logger,
&mockReporter{},
Expand Down
21 changes: 11 additions & 10 deletions pkg/reconciler/inmemorychannel/dispatcher/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,32 +130,33 @@ func NewController(
TrustBundleConfigMapLister: trustBundleConfigMapInformer.Lister().ConfigMaps(system.Namespace()),
}

var globalResync func(obj interface{})

featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store"), func(_ string, _ interface{}) {
if globalResync != nil {
globalResync(nil)
}
})

featureStore.WatchConfigs(cmw)
r := &Reconciler{
multiChannelEventHandler: sh,
reporter: reporter,
messagingClientSet: eventingclient.Get(ctx).MessagingV1(),
eventingClient: eventingclient.Get(ctx).EventingV1beta2(),
eventTypeLister: eventtypeinformer.Get(ctx).Lister(),
eventDispatcher: kncloudevents.NewDispatcher(clientConfig, oidcTokenProvider),
authVerifier: auth.NewVerifier(ctx, eventpolicyinformer.Get(ctx).Lister()),
authVerifier: auth.NewVerifier(ctx, eventpolicyinformer.Get(ctx).Lister(), featureStore.Load()),
clientConfig: clientConfig,
inMemoryChannelLister: inmemorychannelInformer.Lister(),
}

var globalResync func(obj interface{})

featureStore := feature.NewStore(logging.FromContext(ctx).Named("feature-config-store"), func(_ string, _ interface{}) {
if globalResync != nil {
globalResync(nil)
}
})
featureStore.WatchConfigs(cmw)

impl := inmemorychannelreconciler.NewImpl(ctx, r, func(impl *controller.Impl) controller.Options {
return controller.Options{SkipStatusUpdates: true, FinalizerName: finalizerName, ConfigStore: featureStore}
})

globalResync = func(_ interface{}) {
r.authVerifier = auth.NewVerifier(ctx, eventpolicyinformer.Get(ctx).Lister(), featureStore.Load())
impl.GlobalResync(inmemorychannelInformer.Informer())
}

Expand Down

0 comments on commit 3f2dfa6

Please sign in to comment.