feat: apirule: refactor status handling

[Ressetkk]

Status handling in the APIRule controller is one of the problematic pain points during code maintenance. APIRule Controller uses type ReconciliationStatus interface. This iface is then implemented by two structures ReconciliationV2alpha1Status and ReconciliationV1beta1Status.

To access the status function GetStatusBase(string) has to be used. Usually, code uses this function by providing the bogus string value as an argument, eg. s := cmd.GetStatusBase(string(gatewayv1beta1.StatusSkipped)), then we actually modify status using different interface function, eg. s.GenerateStatusFromFailures(failures)

api-gateway/controllers/gateway/apirule_controller.go

Lines 147 to 148 in 5690117

	s := cmd.GetStatusBase(string(gatewayv1beta1.StatusSkipped)).
	GenerateStatusFromFailures(failures)

Then to update the actual status of APIRule, we use err := s.UpdateStatus(&apiRule.Status) function.
Additionally in the code there is another interface that also manages statuses under controllers/status.go. This interface also defines two functions that operate on structures that operate on ReconciliationStatus iface. Not only that, but it also breaks the recommendation of Golang coding practices by defining the interface along with the implementation.

Another problematic implementation is function GetStatusForErrorMap which accepts a map of ResourceSelectors defined in internal/processing/status/status.go. I understand that this function is meant to aggregate the APIRule validation failures, but in the end it doesn't give proper access to aggregated errors, which resulted in a need to introduce small aggregation function #1490.

With the recommendations from the Golang team about the interfaces, and to make a foundation for new reconciliation for APIRule V2, I would like to propose some changes into how to handle the APIRule status.

Proposals

Reduce number of interfaces

Let's take an example of internal/validation/v1beta1/v1beta1.go. The APIRuleValidator uses 3 interfaces accessStrategyValidator mutatorValidator and rulesValidator.

type APIRuleValidator struct {
	ApiRule *gatewayv1beta1.APIRule

	HandlerValidator          handlerValidator
	AccessStrategiesValidator accessStrategyValidator
	MutatorsValidator         mutatorValidator
	InjectionValidator        *validation.InjectionValidator
	RulesValidator            rulesValidator
	ServiceBlockList          map[string][]string
	DomainAllowList           []string
	HostBlockList             []string
	DefaultDomainName         string
}

type accessStrategyValidator interface {
	Validate(attrPath string, accessStrategies []*gatewayv1beta1.Authenticator) []validation.Failure
}

type mutatorValidator interface {
	Validate(attrPath string, rule gatewayv1beta1.Rule) []validation.Failure
}

type rulesValidator interface {
	Validate(attrPath string, rules []gatewayv1beta1.Rule) []validation.Failure
}

Those interfaces are implemented by a single structure. We should not define interfaces if we do not plan on extending them.

Implement errors in Validation handling

Current implementation of Validation makes it hard follow during debugging. The chain for validation goes like this:
Reconciliation.Validate -> APIRuleValidator.Validate -> v1beta1/v2alpha1.Validate.
Each of the validations should return a custom error implementation. Example:

1. Implement ValidationError structure that implements error interface

type Failure struct {
Attribute string
Msg string
}

func (e Failure) Error() string {
return fmt.Sprintf("%s: %s", e.Attribute, e.Msg)
}

Then, instead of returning Failure, we can return error, and treat it like usual error.

If we need to aggregate errors, we should use apimachinery errors.Aggregate interface and NewAggregate() function.

TODOs (subject of change)

• Revisit usage of ReconciliationStatus and remove if deemed unnecessary
• Remove usage of GetStatusBase from ReconciliationCommands and implement proper constructor for Status handler
• Remove usage of ResourceSelector and replace it with separate errors for each of the resource
• Extend validation.Failure{} with error interface implementation
• Adjust the tests without changing their logic