Address review comments

Signed-off-by: claytonig <claytonivorgonsalves@gmail.com>

apis/projectcontour/v1/helpers.go

@@ -51,14 +51,6 @@ func (v *VirtualHost) AuthorizationContext() map[string]string {
 	return nil
 }
 
-// DisableGlobalAuthorization returns true if this virtual host disables
-// global authorization. If a global authorization config is present, the default
-// policy is to not disable.
-func (v *VirtualHost) DisableGlobalAuthorization() bool {
-	return v.Authorization != nil && v.Authorization.GlobalExternalAuthorizationDisabled
-
-}
-
 // GetPrefixReplacements returns replacement prefixes from the path
 // rewrite policy (if any).
 func (r *Route) GetPrefixReplacements() []ReplacePrefix {

apis/projectcontour/v1/httpproxy.go

@@ -193,9 +193,8 @@ type ExtensionServiceReference struct {
 // external authorization GRPC protocol (https://www.envoyproxy.io/docs/envoy/latest/api-v3/service/auth/v3/external_auth.proto).
 type AuthorizationServer struct {
 	// ExtensionServiceRef specifies the extension resource that will authorize client requests.
-	// One of globalExtAuthDisabled or extensionRef must be set.
 	//
-	// +optional
+	// +required
 	ExtensionServiceRef ExtensionServiceReference `json:"extensionRef,omitempty"`
 
 	// AuthPolicy sets a default authorization policy for client requests.
@@ -224,12 +223,6 @@ type AuthorizationServer struct {
 	// WithRequestBody specifies configuration for sending the client request's body to authorization server.
 	// +optional
 	WithRequestBody *AuthorizationServerBufferSettings `json:"withRequestBody,omitempty"`
-
-	// GlobalExternalAuthorizationDisabled optionally disables the global external authorization on the virtual host.
-	// One of globalExtAuthDisabled or extensionRef must be set.
-	//
-	// +optional
-	GlobalExternalAuthorizationDisabled bool `json:"globalExtAuthDisabled,omitempty"`
 }
 
 // AuthorizationServerBufferSettings enables ExtAuthz filter to buffer client request data and send it as part of authorization request

apis/projectcontour/v1alpha1/contourconfig.go

@@ -65,7 +65,7 @@ type ContourConfigurationSpec struct {
 	// GlobalExternalAuthorization allows envoys external authorization filter
 	// to be enabled for all virtual hosts.
 	// +optional
-	GlobalExternalAuthorization *GlobalExternalAuthorizationConfig `json:"globalExtAuth,omitempty"`
+	GlobalExternalAuthorization *contour_api_v1.AuthorizationServer `json:"globalExtAuth,omitempty"`
 
 	// RateLimitService optionally holds properties of the Rate Limit Service
 	// to be used for global rate limiting.
@@ -628,67 +628,6 @@ type NetworkParameters struct {
 	EnvoyAdminPort *int `json:"adminPort,omitempty"`
 }
 
-// GlobalExternalAuthorizationConfig defines properties of global external authorization.
-type GlobalExternalAuthorizationConfig struct {
-	// ExtensionService identifies the extension service responsible for the authorization.
-	// formatted as <namespace>/<name>.
-	ExtensionService string `json:"extensionService,omitempty"`
-	// AuthPolicy sets a default authorization policy for client requests.
-	// This policy will be used unless overridden by individual routes.
-	//
-	// +optional
-	AuthPolicy *GlobalAuthorizationPolicy `json:"globalAuthPolicy,omitempty"`
-	// ResponseTimeout configures maximum time to wait for a check response from the authorization server.
-	// Timeout durations are expressed in the Go [Duration format](https://godoc.org/time#ParseDuration).
-	// Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
-	// The string "infinity" is also a valid input and specifies no timeout.
-	//
-	// +optional
-	ResponseTimeout string `json:"responseTimeout,omitempty"`
-	// If FailOpen is true, the client request is forwarded to the upstream service
-	// even if the authorization server fails to respond. This field should not be
-	// set in most cases. It is intended for use only while migrating applications
-	// from internal authorization to Contour external authorization.
-	//
-	// +optional
-	FailOpen *bool `json:"failOpen,omitempty"`
-	// WithRequestBody specifies configuration for sending the client request's body to authorization server.
-	// +optional
-	WithRequestBody *GlobalAuthorizationServerBufferSettings `json:"withRequestBody,omitempty"`
-}
-
-// GlobalAuthorizationServerBufferSettings enables ExtAuthz filter to buffer client request data and send it as part of authorization request
-type GlobalAuthorizationServerBufferSettings struct {
-	// MaxRequestBytes sets the maximum size of message body ExtAuthz filter will hold in-memory.
-	// +optional
-	// +kubebuilder:validation:Minimum=1
-	// +kubebuilder:default=1024
-	MaxRequestBytes uint32 `json:"maxRequestBytes,omitempty"`
-	// If AllowPartialMessage is true, then Envoy will buffer the body until MaxRequestBytes are reached.
-	// +optional
-	AllowPartialMessage *bool `json:"allowPartialMessage,omitempty"`
-	// If PackAsBytes is true, the body sent to Authorization Server is in raw bytes.
-	// +optional
-	PackAsBytes *bool `json:"packAsBytes,omitempty"`
-}
-
-// GlobalAuthorizationPolicy modifies how client requests are authenticated.
-type GlobalAuthorizationPolicy struct {
-	// When true, this field disables client request authentication
-	// for the scope of the policy.
-	//
-	// +optional
-	Disabled *bool `json:"disabled,omitempty"`
-	// Context is a set of key/value pairs that are sent to the
-	// authentication server in the check request. If a context
-	// is provided at an enclosing scope, the entries are merged
-	// such that the inner scope overrides matching keys from the
-	// outer scope.
-	//
-	// +optional
-	Context map[string]string `json:"context,omitempty"`
-}
-
 // RateLimitServiceConfig defines properties of a global Rate Limit Service.
 type RateLimitServiceConfig struct {
 	// ExtensionService identifies the extension service defining the RLS.

cmd/contour/serve.go

@@ -648,10 +648,10 @@ func (s *Server) setupGlobalExternalAuthentication(contourConfiguration contour_
 
 	// ensure the specified ExtensionService exists
 	extensionSvc := &contour_api_v1alpha1.ExtensionService{}
-	namespaceName := k8s.NamespacedNameFrom(contourConfiguration.GlobalExternalAuthorization.ExtensionService)
+
 	key := client.ObjectKey{
-		Namespace: namespaceName.Namespace,
-		Name:      namespaceName.Name,
+		Namespace: contourConfiguration.GlobalExternalAuthorization.ExtensionServiceRef.Namespace,
+		Name:      contourConfiguration.GlobalExternalAuthorization.ExtensionServiceRef.Name,
 	}
 
 	// Using GetAPIReader() here because the manager's caches won't be started yet,
@@ -676,12 +676,17 @@ func (s *Server) setupGlobalExternalAuthentication(contourConfiguration contour_
 		sni = extensionSvc.Spec.UpstreamValidation.SubjectName
 	}
 
+	var context map[string]string
+	if contourConfiguration.GlobalExternalAuthorization.AuthPolicy.Context != nil {
+		context = contourConfiguration.GlobalExternalAuthorization.AuthPolicy.Context
+	}
+
 	return &xdscache_v3.GlobalExternalAuthConfig{
 		ExtensionService: key,
 		SNI:              sni,
 		Timeout:          responseTimeout,
-		FailOpen:         ref.Val(contourConfiguration.GlobalExternalAuthorization.FailOpen, false),
-		Context:          contourConfiguration.GlobalExternalAuthorization.AuthPolicy.Context,
+		FailOpen:         contourConfiguration.GlobalExternalAuthorization.FailOpen,
+		Context:          context,
 	}, nil
 }
 
@@ -898,7 +903,7 @@ type dagBuilderConfig struct {
 	connectTimeout                     time.Duration
 	client                             client.Client
 	metrics                            *metrics.Metrics
-	globalExternalAuthorizationService *contour_api_v1alpha1.GlobalExternalAuthorizationConfig
+	globalExternalAuthorizationService *contour_api_v1.AuthorizationServer
 }
 
 func (s *Server) getDAGBuilder(dbc dagBuilderConfig) *dag.Builder {

cmd/contour/servecontext.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 	"time"
 
+	contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
 	contour_api_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
 	envoy_v3 "github.com/projectcontour/contour/internal/envoy/v3"
 	"github.com/projectcontour/contour/internal/k8s"
@@ -388,27 +389,30 @@ func (ctx *serveContext) convertToContourConfigurationSpec() contour_api_v1alpha
 		serverHeaderTransformation = contour_api_v1alpha1.PassThroughServerHeader
 	}
 
-	var globalExtAuth *contour_api_v1alpha1.GlobalExternalAuthorizationConfig
+	var globalExtAuth *contour_api_v1.AuthorizationServer
 	if ctx.Config.GlobalExternalAuthorization.ExtensionService != "" {
-
-		globalExtAuth = &contour_api_v1alpha1.GlobalExternalAuthorizationConfig{
-			ExtensionService: ctx.Config.GlobalExternalAuthorization.ExtensionService,
-			ResponseTimeout:  ctx.Config.GlobalExternalAuthorization.ResponseTimeout,
-			FailOpen:         ref.To(ctx.Config.GlobalExternalAuthorization.FailOpen),
+		nsedName := k8s.NamespacedNameFrom(ctx.Config.GlobalExternalAuthorization.ExtensionService)
+		globalExtAuth = &contour_api_v1.AuthorizationServer{
+			ExtensionServiceRef: contour_api_v1.ExtensionServiceReference{
+				Name:      nsedName.Name,
+				Namespace: nsedName.Namespace,
+			},
+			ResponseTimeout: ctx.Config.GlobalExternalAuthorization.ResponseTimeout,
+			FailOpen:        ctx.Config.GlobalExternalAuthorization.FailOpen,
 		}
 
 		if ctx.Config.GlobalExternalAuthorization.AuthPolicy != nil {
-			globalExtAuth.AuthPolicy = &contour_api_v1alpha1.GlobalAuthorizationPolicy{
-				Disabled: ref.To(ctx.Config.GlobalExternalAuthorization.AuthPolicy.Disabled),
+			globalExtAuth.AuthPolicy = &contour_api_v1.AuthorizationPolicy{
+				Disabled: ctx.Config.GlobalExternalAuthorization.AuthPolicy.Disabled,
 				Context:  ctx.Config.GlobalExternalAuthorization.AuthPolicy.Context,
 			}
 		}
 
 		if ctx.Config.GlobalExternalAuthorization.WithRequestBody != nil {
-			globalExtAuth.WithRequestBody = &contour_api_v1alpha1.GlobalAuthorizationServerBufferSettings{
+			globalExtAuth.WithRequestBody = &contour_api_v1.AuthorizationServerBufferSettings{
 				MaxRequestBytes:     ctx.Config.GlobalExternalAuthorization.WithRequestBody.MaxRequestBytes,
-				AllowPartialMessage: ref.To(ctx.Config.GlobalExternalAuthorization.WithRequestBody.AllowPartialMessage),
-				PackAsBytes:         ref.To(ctx.Config.GlobalExternalAuthorization.WithRequestBody.PackAsBytes),
+				AllowPartialMessage: ctx.Config.GlobalExternalAuthorization.WithRequestBody.AllowPartialMessage,
+				PackAsBytes:         ctx.Config.GlobalExternalAuthorization.WithRequestBody.PackAsBytes,
 			}
 		}
 	}

cmd/contour/servecontext_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/projectcontour/contour/pkg/config"
 	"github.com/tsaarni/certyaml"
 
+	contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
 	contour_api_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
 	"github.com/projectcontour/contour/internal/contourconfig"
 	envoy_v3 "github.com/projectcontour/contour/internal/envoy/v3"
@@ -719,19 +720,22 @@ func TestConvertServeContext(t *testing.T) {
 				return ctx
 			},
 			getContourConfiguration: func(cfg contour_api_v1alpha1.ContourConfigurationSpec) contour_api_v1alpha1.ContourConfigurationSpec {
-				cfg.GlobalExternalAuthorization = &contour_api_v1alpha1.GlobalExternalAuthorizationConfig{
-					ExtensionService: "extauthns/extauthtext",
-					FailOpen:         ref.To(true),
-					AuthPolicy: &contour_api_v1alpha1.GlobalAuthorizationPolicy{
+				cfg.GlobalExternalAuthorization = &contour_api_v1.AuthorizationServer{
+					ExtensionServiceRef: contour_api_v1.ExtensionServiceReference{
+						Name:      "extauthtext",
+						Namespace: "extauthns",
+					},
+					FailOpen: true,
+					AuthPolicy: &contour_api_v1.AuthorizationPolicy{
 						Context: map[string]string{
 							"foo": "bar",
 						},
-						Disabled: ref.To(false),
+						Disabled: false,
 					},
-					WithRequestBody: &contour_api_v1alpha1.GlobalAuthorizationServerBufferSettings{
+					WithRequestBody: &contour_api_v1.AuthorizationServerBufferSettings{
 						MaxRequestBytes:     512,
-						PackAsBytes:         ref.To(true),
-						AllowPartialMessage: ref.To(true),
+						PackAsBytes:         true,
+						AllowPartialMessage: true,
 					},
 				}
 				return cfg