diff --git a/controllers/auth_config_controller.go b/controllers/auth_config_controller.go index 6c2b96c9..c9f5d8de 100644 --- a/controllers/auth_config_controller.go +++ b/controllers/auth_config_controller.go @@ -58,12 +58,13 @@ const ( // AuthConfigReconciler reconciles an AuthConfig object type AuthConfigReconciler struct { client.Client - Logger logr.Logger - Scheme *runtime.Scheme - Index index.Index - StatusReport *StatusReportMap - LabelSelector labels.Selector - Namespace string + Logger logr.Logger + Scheme *runtime.Scheme + Index index.Index + AllowSupersedingHostSubsets bool + StatusReport *StatusReportMap + LabelSelector labels.Selector + Namespace string indexBootstrap sync.Mutex } @@ -608,7 +609,7 @@ func (r *AuthConfigReconciler) addToIndex(ctx context.Context, resourceNamespace for _, host := range hosts { // check for host name collision between resources - if indexedResourceId, found := r.Index.FindId(host); found && indexedResourceId != resourceId { + if r.hostTaken(host, resourceId) { looseHosts = append(looseHosts, host) logger.Info("host already taken", "host", host) continue @@ -625,6 +626,15 @@ func (r *AuthConfigReconciler) addToIndex(ctx context.Context, resourceNamespace return } +func (r *AuthConfigReconciler) hostTaken(host, resourceId string) bool { + indexedResourceId, found := r.Index.FindId(host) + return found && indexedResourceId != resourceId && !r.supersedeHostSubset(host, indexedResourceId) +} + +func (r *AuthConfigReconciler) supersedeHostSubset(host, supersetResourceId string) bool { + return r.AllowSupersedingHostSubsets && !utils.SliceContains(r.Index.FindKeys(supersetResourceId), host) +} + func (r *AuthConfigReconciler) bootstrapIndex(ctx context.Context) error { r.indexBootstrap.Lock() defer r.indexBootstrap.Unlock() diff --git a/controllers/auth_config_controller_test.go b/controllers/auth_config_controller_test.go index 7cf3c77b..08e54abf 100644 --- a/controllers/auth_config_controller_test.go +++ b/controllers/auth_config_controller_test.go @@ -230,7 +230,7 @@ func TestTranslateAuthConfig(t *testing.T) { // TODO } -func TestPreventHostCollision(t *testing.T) { +func TestPreventHostCollisionExactMatches(t *testing.T) { mockController := gomock.NewController(t) defer mockController.Finish() indexMock := mock_index.NewMockIndex(mockController) @@ -242,12 +242,13 @@ func TestPreventHostCollision(t *testing.T) { client := newTestK8sClient(&authConfig, &secret) reconciler := newTestAuthConfigReconciler(client, indexMock) - indexMock.EXPECT().Empty().Return(false) - indexMock.EXPECT().FindKeys(authConfigName.String()).Return([]string{}).AnyTimes() - indexMock.EXPECT().FindId("echo-api").Return("other-namespace/other-auth-config-with-same-host", true) - indexMock.EXPECT().FindId("other.io").Return("", false) - indexMock.EXPECT().FindId("yet-another.io").Return(fmt.Sprintf("%s/other-auth-config-same-ns", authConfig.Namespace), true) - indexMock.EXPECT().Set(authConfigName.String(), "other.io", gomock.Any(), true) + indexMock.EXPECT().Empty().Return(false) // simulate index not empty, so it skips bootstraping + indexMock.EXPECT().FindKeys(authConfigName.String()).Return([]string{}).AnyTimes() // simulate no prexisting hosts linked to the authconfig to be reconciled + indexMock.EXPECT().FindId("echo-api").Return("other-namespace/other-auth-config-with-same-host", true) // simulate other existing authconfig with conflicting host, in a different namespace + indexMock.EXPECT().FindId("other.io").Return(fmt.Sprintf("%s/other-auth-config-same-ns", authConfig.Namespace), true) // simulate other existing authconfig with conflicting host, in the same namespace + indexMock.EXPECT().FindId("yet-another.io").Return("", false) // simulate no other existing authconfig with conflicting host + + indexMock.EXPECT().Set(authConfigName.String(), "yet-another.io", gomock.Any(), true) // expect only the new host to be indexed result, err := reconciler.Reconcile(context.Background(), reconcile.Request{NamespacedName: authConfigName}) @@ -255,6 +256,53 @@ func TestPreventHostCollision(t *testing.T) { assert.NilError(t, err) } +func TestPreventHostCollisionAllowSupersedingHostSubsets(t *testing.T) { + mockController := gomock.NewController(t) + defer mockController.Finish() + indexMock := mock_index.NewMockIndex(mockController) + + authConfig := newTestAuthConfig(map[string]string{}) + authConfig.Spec.Hosts = []string{"echo-api.io"} + authConfigName := types.NamespacedName{Name: authConfig.Name, Namespace: authConfig.Namespace} + + secret := newTestOAuthClientSecret() + client := newTestK8sClient(&authConfig, &secret) + reconciler := newTestAuthConfigReconciler(client, indexMock) + + indexMock.EXPECT().Empty().Return(false).AnyTimes() // simulate index not empty, so it skips bootstraping + indexMock.EXPECT().FindKeys(authConfigName.String()).Return([]string{}).AnyTimes() // simulate no prexisting hosts linked to the authconfig to be reconciled + + // allow superseding host subsets = false + indexMock.EXPECT().FindId("echo-api.io").Return("other/other", true) // simulate other existing authconfig with conflicting host + + result, err := reconciler.Reconcile(context.Background(), reconcile.Request{NamespacedName: authConfigName}) + + assert.DeepEqual(t, result, ctrl.Result{}) + assert.NilError(t, err) + + // allow superseding host subsets = true, conflicting host found and the new one is NOT a strict subset of the one found + reconciler.AllowSupersedingHostSubsets = true + indexMock.EXPECT().FindId("echo-api.io").Return("other/other-1", true) // simulate other existing authconfig with conflicting host + indexMock.EXPECT().FindKeys("other/other-1").Return([]string{"echo-api.io"}) // simulate identical host found linked to other authconfig (i.e. not a strict subset) + + result, err = reconciler.Reconcile(context.Background(), reconcile.Request{NamespacedName: authConfigName}) + + assert.DeepEqual(t, result, ctrl.Result{}) + assert.NilError(t, err) + + // allow superseding host subsets = true, conflicting host found but the new one is a strict subset of the one found + reconciler.AllowSupersedingHostSubsets = true + indexMock.EXPECT().FindId("echo-api.io").Return("other/other-2", true) // simulate other existing authconfig with conflicting host + indexMock.EXPECT().FindKeys("other/other-2").Return([]string{"*.io"}) // simulate superset host found linked to other authconfig + + indexMock.EXPECT().Set(authConfigName.String(), "echo-api.io", gomock.Any(), true) // expect only the new host to be indexed + + result, err = reconciler.Reconcile(context.Background(), reconcile.Request{NamespacedName: authConfigName}) + + assert.DeepEqual(t, result, ctrl.Result{}) + assert.NilError(t, err) +} + func TestMissingWatchedAuthConfigLabels(t *testing.T) { mockController := gomock.NewController(t) defer mockController.Finish() diff --git a/docs/architecture.md b/docs/architecture.md index b7e508b8..d65589a4 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -238,6 +238,8 @@ Authorino tries to prevent host name collision between `AuthConfig`s by rejectin When wildcards are involved, a host name that matches a host wildcard already linked in the index to another `AuthConfig` will be considered taken, and therefore the newest `AuthConfig` will be rejected to be linked to that host. +This behavior can be disabled to allow `AuthConfig`s to partially supersede each others' host names (limited to strict host subsets), by supplying the `--allow-superseding-host-subsets` command-line flag when running the Authorino instance. + ## The Authorization JSON On every Auth Pipeline, Authorino builds the **Authorization JSON**, a "working-memory" data structure composed of `context` (information about the request, as supplied by the Envoy proxy to Authorino) and `auth` (objects resolved in phases (i) to (v) of the pipeline). The evaluators of each phase can read from the Authorization JSON and implement dynamic properties and decisions based on its values. diff --git a/main.go b/main.go index ddddc067..f33a43c0 100644 --- a/main.go +++ b/main.go @@ -104,6 +104,7 @@ type authServerOptions struct { watchNamespace string watchedAuthConfigLabelSelector string watchedSecretLabelSelector string + allowSupersedingHostSubsets bool timeout int extAuthGRPCPort int extAuthHTTPPort int @@ -165,6 +166,7 @@ func authServerCmd(opts *authServerOptions) *cobra.Command { cmd.PersistentFlags().StringVar(&opts.watchNamespace, "watch-namespace", utils.EnvVar("WATCH_NAMESPACE", ""), "Kubernetes namespace to watch") cmd.PersistentFlags().StringVar(&opts.watchedAuthConfigLabelSelector, "auth-config-label-selector", utils.EnvVar("AUTH_CONFIG_LABEL_SELECTOR", ""), "Kubernetes label selector to filter AuthConfig resources to watch") cmd.PersistentFlags().StringVar(&opts.watchedSecretLabelSelector, "secret-label-selector", utils.EnvVar("SECRET_LABEL_SELECTOR", "authorino.kuadrant.io/managed-by=authorino"), "Kubernetes label selector to filter Secret resources to watch") + cmd.PersistentFlags().BoolVar(&opts.allowSupersedingHostSubsets, "allow-superseding-host-subsets", false, "Enable AuthConfigs to supersede strict host subsets of supersets already taken") cmd.PersistentFlags().IntVar(&opts.timeout, "timeout", utils.EnvVar("TIMEOUT", 0), "Server timeout - in milliseconds") cmd.PersistentFlags().IntVar(&opts.extAuthGRPCPort, "ext-auth-grpc-port", utils.EnvVar("EXT_AUTH_GRPC_PORT", 50051), "Port number of authorization server - gRPC interface") cmd.PersistentFlags().IntVar(&opts.extAuthHTTPPort, "ext-auth-http-port", utils.EnvVar("EXT_AUTH_HTTP_PORT", 5001), "Port number of authorization server - raw HTTP interface") @@ -256,13 +258,14 @@ func runAuthorizationServer(cmd *cobra.Command, _ []string) { // sets up the authconfig reconciler authConfigReconciler := &controllers.AuthConfigReconciler{ - Client: mgr.GetClient(), - Index: index, - StatusReport: statusReport, - Logger: controllerLogger.WithName("authconfig"), - Scheme: mgr.GetScheme(), - LabelSelector: controllers.ToLabelSelector(opts.watchedAuthConfigLabelSelector), - Namespace: opts.watchNamespace, + Client: mgr.GetClient(), + Index: index, + AllowSupersedingHostSubsets: opts.allowSupersedingHostSubsets, + StatusReport: statusReport, + Logger: controllerLogger.WithName("authconfig"), + Scheme: mgr.GetScheme(), + LabelSelector: controllers.ToLabelSelector(opts.watchedAuthConfigLabelSelector), + Namespace: opts.watchNamespace, } if err = authConfigReconciler.SetupWithManager(mgr); err != nil { logger.Error(err, "failed to setup controller", "controller", "authconfig") diff --git a/pkg/index/index_test.go b/pkg/index/index_test.go index 76ca6a30..1769c3f1 100644 --- a/pkg/index/index_test.go +++ b/pkg/index/index_test.go @@ -13,34 +13,28 @@ import ( // TestAuthConfigTree tests operations to build and modify the following index tree: // -// ┌───┐ -// ┌─────────┤ . ├──────────┐ -// │ └───┘ │ -// │ │ -// │ │ -// ┌──┴─┐ ┌──┴──┐ -// ┌───┤ io ├───┐ ┌───┤ com ├───┐ -// │ └────┘ │ │ └─────┘ │ -// │ │ │ │ -// │ │ │ │ -// │ │ │ │ -// -// ┌─┴─┐ ┌──┴──┐ ┌───┴──┐ ┌───┴──┐ -// │ * │ │ nip │ │ pets │ ┌─┤ acme ├─┐ -// └───┘ └──┬──┘ └───┬──┘ │ └──────┘ │ -// -// ▲ │ │ │ │ -// │ │ │ │ │ -// │ │ │ │ │ -// │ ┌─────┴──────┐ ┌─┴─┐ ┌──┴──┐ ┌─┴─┐ -// -// auth-1 │ talker-api │ │ * │ │ api │ │ * │ -// -// └────────────┘ └───┘ └─────┘ └───┘ -// ▲ ▲ ▲ ▲ -// │ │ │ │ -// │ │ │ │ -// └───auth-2──┘ auth-3 auth-4 +// ┌───┐ +// ┌─────────┤ . ├────────┐ +// │ └───┘ │ +// │ │ +// │ │ +// ┌──┴─┐ ┌──┴──┐ +// ┌───┤ io ├──┐ ┌───┤ com ├───┐ +// │ └────┘ │ │ └─────┘ │ +// │ │ │ │ +// │ │ │ │ +// ┌─┴─┐ ┌──┴──┐ ┌──┴───┐ ┌───┴──┐ +// │ * │ │ nip │ │ pets │ ┌─┤ acme ├─┐ +// └───┘ └──┬──┘ └──┬───┘ │ └──────┘ │ +// ▲ │ │ │ │ +// │ │ │ │ │ +// │ ┌──────┴─────┐ ┌─┴─┐ ┌──┴──┐ ┌─┴─┐ +// auth-1 │ talker-api │ │ * │ │ api │ │ * │ +// └────────────┘ └───┘ └─────┘ └───┘ +// ▲ ▲ ▲ ▲ +// │ │ │ │ +// │ │ │ │ +// └──auth-2──┘ auth-3 auth-4 func TestAuthConfigTree(t *testing.T) { c := newAuthConfigTree() @@ -50,22 +44,26 @@ func TestAuthConfigTree(t *testing.T) { authConfig4 := buildTestAuthConfig() // Build the index + // Set the more generic host first if err := c.Set("auth-1", "*.io", authConfig1, false); err != nil { t.Error(err) } - if err := c.Set("auth-2", "*.pets.com", authConfig2, false); err != nil { + // ...and then the more specific one + if err := c.Set("auth-2", "talker-api.nip.io", authConfig2, false); err != nil { t.Error(err) } - if err := c.Set("auth-2", "talker-api.nip.io", authConfig2, false); err != nil { + if err := c.Set("auth-2", "*.pets.com", authConfig2, false); err != nil { t.Error(err) } + // Set the more specific host first if err := c.Set("auth-3", "api.acme.com", authConfig3, false); err != nil { t.Error(err) } + // ...and then the more generic one if err := c.Set("auth-4", "*.acme.com", authConfig4, false); err != nil { t.Error(err) } @@ -130,7 +128,7 @@ func TestAuthConfigTree(t *testing.T) { assert.Check(t, config == nil) config = c.Get("talker-api.nip.io") - assert.DeepEqual(t, *config, authConfig1) // because `*.io -> auth-1` is still in the tree + assert.DeepEqual(t, *config, authConfig1) // because `*.io <- auth-1` is still in the tree config = c.Get("api.acme.com") assert.DeepEqual(t, *config, authConfig3) @@ -138,7 +136,7 @@ func TestAuthConfigTree(t *testing.T) { c.Delete("auth-3") config = c.Get("api.acme.com") - assert.DeepEqual(t, *config, authConfig4) // because `*.acme.com -> auth-4` is still in the tree + assert.DeepEqual(t, *config, authConfig4) // because `*.acme.com <- auth-4` is still in the tree } type bogusIdentity struct{}