Skip to content

Commit

Permalink
refactor: remove unneccessary getters
Browse files Browse the repository at this point in the history
  • Loading branch information
KevFan committed Apr 2, 2024
1 parent 00a0343 commit 4e8d06e
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 64 deletions.
38 changes: 13 additions & 25 deletions api/v1beta2/authpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,12 @@ type AuthPolicyCommonSpec struct {
AuthScheme *AuthSchemeSpec `json:"rules,omitempty"`
}

// GetRouteSelectors returns the top-level route selectors of the auth scheme.
// impl: RouteSelectorsGetter
func (c AuthPolicyCommonSpec) GetRouteSelectors() []RouteSelector {
return c.RouteSelectors
}

type AuthPolicyStatus struct {
// ObservedGeneration reflects the generation of the most recently observed spec.
// +optional
Expand Down Expand Up @@ -279,7 +285,7 @@ func (ap *AuthPolicy) GetRulesHostnames() (ruleHosts []string) {
}
}

appendCommonSpecRuleHosts := func(c AuthPolicyCommonSpec) {
appendCommonSpecRuleHosts := func(c *AuthPolicyCommonSpec) {
if c.AuthScheme == nil {
return
}
Expand All @@ -306,8 +312,8 @@ func (ap *AuthPolicy) GetRulesHostnames() (ruleHosts []string) {
}
}

appendRuleHosts(ap)
appendCommonSpecRuleHosts(ap.GetCommonSpec())
appendRuleHosts(ap.Spec.CommonSpec())
appendCommonSpecRuleHosts(ap.Spec.CommonSpec())

return
}
Expand All @@ -324,30 +330,12 @@ func (ap *AuthPolicy) DirectReferenceAnnotationName() string {
return AuthPolicyDirectReferenceAnnotationName
}

func (ap *AuthPolicy) GetCommonSpec() AuthPolicyCommonSpec {
if ap.Spec.Defaults != nil {
return *ap.Spec.Defaults
func (ap *AuthPolicySpec) CommonSpec() *AuthPolicyCommonSpec {
if ap.Defaults != nil {
return ap.Defaults
}

return ap.Spec.AuthPolicyCommonSpec
}

func (ap *AuthPolicy) GetNamedPatterns() map[string]authorinoapi.PatternExpressions {
return ap.GetCommonSpec().NamedPatterns
}

func (ap *AuthPolicy) GetConditions() []authorinoapi.PatternExpressionOrRef {
return ap.GetCommonSpec().Conditions
}

func (ap *AuthPolicy) GetAuthScheme() *AuthSchemeSpec {
return ap.GetCommonSpec().AuthScheme
}

// GetRouteSelectors returns the top-level route selectors of the auth scheme.
// impl: RouteSelectorsGetter
func (ap *AuthPolicy) GetRouteSelectors() []RouteSelector {
return ap.GetCommonSpec().RouteSelectors
return &ap.AuthPolicyCommonSpec
}

//+kubebuilder:object:root=true
Expand Down
8 changes: 4 additions & 4 deletions api/v1beta2/authpolicy_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ func TestCommonAuthRuleSpecGetRouteSelectors(t *testing.T) {
}

func TestAuthPolicySpecGetRouteSelectors(t *testing.T) {
p := &AuthPolicy{}
if p.GetRouteSelectors() != nil {
spec := &AuthPolicySpec{}
if spec.GetRouteSelectors() != nil {
t.Errorf("Expected nil route selectors")
}
routeSelector := testBuildRouteSelector()
p.Spec.RouteSelectors = []RouteSelector{routeSelector}
result := p.GetRouteSelectors()
spec.RouteSelectors = []RouteSelector{routeSelector}
result := spec.GetRouteSelectors()
if len(result) != 1 {
t.Errorf("Expected 1 route selector, got %d", len(result))
}
Expand Down
37 changes: 23 additions & 14 deletions controllers/authpolicy_authconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,40 +111,47 @@ func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *api.Au
// hosts
authConfig.Spec.Hosts = hosts

commonSpec := ap.Spec.CommonSpec()

// named patterns
if namedPatterns := ap.GetNamedPatterns(); len(namedPatterns) > 0 {
if namedPatterns := commonSpec.NamedPatterns; len(namedPatterns) > 0 {
authConfig.Spec.NamedPatterns = namedPatterns
}

// top-level conditions
topLevelConditionsFromRouteSelectors, err := authorinoConditionsFromRouteSelectors(route, ap)
topLevelConditionsFromRouteSelectors, err := authorinoConditionsFromRouteSelectors(route, commonSpec)
if err != nil {
return nil, err
}
if len(topLevelConditionsFromRouteSelectors) == 0 {
topLevelConditionsFromRouteSelectors = authorinoConditionsFromHTTPRoute(route)
}
if len(topLevelConditionsFromRouteSelectors) > 0 || len(ap.GetConditions()) > 0 {
authConfig.Spec.Conditions = append(ap.GetConditions(), topLevelConditionsFromRouteSelectors...)
if len(topLevelConditionsFromRouteSelectors) > 0 || len(commonSpec.Conditions) > 0 {
authConfig.Spec.Conditions = append(commonSpec.Conditions, topLevelConditionsFromRouteSelectors...)
}

// return early if authScheme is nil
if commonSpec.AuthScheme == nil {
return authConfig, nil
}

// authentication
if authentication := ap.GetAuthScheme().Authentication; len(authentication) > 0 {
if authentication := commonSpec.AuthScheme.Authentication; len(authentication) > 0 {
authConfig.Spec.Authentication = authorinoSpecsFromConfigs(authentication, func(config api.AuthenticationSpec) authorinoapi.AuthenticationSpec { return config.AuthenticationSpec })
}

// metadata
if metadata := ap.GetAuthScheme().Metadata; len(metadata) > 0 {
if metadata := commonSpec.AuthScheme.Metadata; len(metadata) > 0 {
authConfig.Spec.Metadata = authorinoSpecsFromConfigs(metadata, func(config api.MetadataSpec) authorinoapi.MetadataSpec { return config.MetadataSpec })
}

// authorization
if authorization := ap.GetAuthScheme().Authorization; len(authorization) > 0 {
if authorization := commonSpec.AuthScheme.Authorization; len(authorization) > 0 {
authConfig.Spec.Authorization = authorinoSpecsFromConfigs(authorization, func(config api.AuthorizationSpec) authorinoapi.AuthorizationSpec { return config.AuthorizationSpec })
}

// response
if response := ap.GetAuthScheme().Response; response != nil {
if response := commonSpec.AuthScheme.Response; response != nil {
authConfig.Spec.Response = &authorinoapi.ResponseSpec{
Unauthenticated: response.Unauthenticated,
Unauthorized: response.Unauthorized,
Expand All @@ -160,7 +167,7 @@ func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *api.Au
}

// callbacks
if callbacks := ap.GetAuthScheme().Callbacks; len(callbacks) > 0 {
if callbacks := commonSpec.AuthScheme.Callbacks; len(callbacks) > 0 {
authConfig.Spec.Callbacks = authorinoSpecsFromConfigs(callbacks, func(config api.CallbackSpec) authorinoapi.CallbackSpec { return config.CallbackSpec })
}

Expand All @@ -187,8 +194,10 @@ func authorinoSpecsFromConfigs[T, U any](configs map[string]U, extractAuthorinoS
}

func mergeConditionsFromRouteSelectorsIntoConfigs(ap *api.AuthPolicy, route *gatewayapiv1.HTTPRoute, authConfig *authorinoapi.AuthConfig) (*authorinoapi.AuthConfig, error) {
commonSpec := ap.Spec.CommonSpec()

// authentication
for name, config := range ap.GetAuthScheme().Authentication {
for name, config := range commonSpec.AuthScheme.Authentication {
conditions, err := authorinoConditionsFromRouteSelectors(route, config)
if err != nil {
return nil, err
Expand All @@ -202,7 +211,7 @@ func mergeConditionsFromRouteSelectorsIntoConfigs(ap *api.AuthPolicy, route *gat
}

// metadata
for name, config := range ap.GetAuthScheme().Metadata {
for name, config := range commonSpec.AuthScheme.Metadata {
conditions, err := authorinoConditionsFromRouteSelectors(route, config)
if err != nil {
return nil, err
Expand All @@ -216,7 +225,7 @@ func mergeConditionsFromRouteSelectorsIntoConfigs(ap *api.AuthPolicy, route *gat
}

// authorization
for name, config := range ap.GetAuthScheme().Authorization {
for name, config := range commonSpec.AuthScheme.Authorization {
conditions, err := authorinoConditionsFromRouteSelectors(route, config)
if err != nil {
return nil, err
Expand All @@ -230,7 +239,7 @@ func mergeConditionsFromRouteSelectorsIntoConfigs(ap *api.AuthPolicy, route *gat
}

// response
if response := ap.GetAuthScheme().Response; response != nil {
if response := commonSpec.AuthScheme.Response; response != nil {
// response success headers
for name, config := range response.Success.Headers {
conditions, err := authorinoConditionsFromRouteSelectors(route, config)
Expand Down Expand Up @@ -261,7 +270,7 @@ func mergeConditionsFromRouteSelectorsIntoConfigs(ap *api.AuthPolicy, route *gat
}

// callbacks
for name, config := range ap.GetAuthScheme().Callbacks {
for name, config := range commonSpec.AuthScheme.Callbacks {
conditions, err := authorinoConditionsFromRouteSelectors(route, config)
if err != nil {
return nil, err
Expand Down
Loading

0 comments on commit 4e8d06e

Please sign in to comment.