THREESCALE-8467 (1): Granular options provider logic

[sergioifg94]

The GetHighAvailabilityOptions method that is used by all external component reconcilers has logic that expects the secrets for all the components being external. This means that even when a dependency is configured as internal, as long as another dependency is configured external this logic will try to retrieve the secret for the internal dependency, failing the reconciliation.

Fix this by making the logic only populate the options for the dependencies configured as external.

Fixes https://issues.redhat.com/browse/THREESCALE-8467

[sergioifg94]

/test test-e2e

[eguzki]

The code fix looks goods

I will approve when I test it locally

[eguzki]

Just for curiosity, it the following approach equivalent?

setOptionsFns := []func() error{}                                         
                                                                          
if h.apimanager.IsExternal(appsv1alpha1.BackendRedis) {                   
    setOptionsFns = append(setOptionsFns, h.setBackendRedisOptions)       
}                                                                         
if h.apimanager.IsExternal(appsv1alpha1.SystemRedis) {                    
    setOptionsFns = append(setOptionsFns, h.setSystemRedisOptions)        
}                                                                         
if h.apimanager.IsExternal(appsv1alpha1.SystemDatabase) {                 
    setOptionsFns = append(setOptionsFns, h.setSystemDatabaseOptions)     
}                                                                         
                                                                          
for _, setOptions := range setOptionsFns {                                
    if err := setOptions(); err != nil {                                  
        return nil, err                                                   
    }                                                                     
}                                                                         

In case they are equivalents, does you approach have any added benefit?

[eguzki]

local test with the following APIManager fails

externalComponents:
  system:
    database: true

The operator log:

2022-06-10T23:25:57.425+0200	ERROR	controller	Reconciler error	{"reconcilerGroup": "apps.3scale.net", "reconcilerKind": "APIManager", "controller": "apimanager", "name": "apimanager1", "namespace": "eguzki", "error": "Key: 'HighAvailabilityOptions.BackendRedisQueuesEndpoint' Error:Field validation for 'BackendRedisQueuesEndpoint' failed on the 'required' tag\nKey: 'HighAvailabilityOptions.BackendRedisStorageEndpoint' Error:Field validation for 'BackendRedisStorageEndpoint' failed on the 'required' tag\nKey: 'HighAvailabilityOptions.SystemRedisURL' Error:Field validation for 'SystemRedisURL' failed on the 'required' tag"}

The struct validation tags should be removed to make all of them optional at

3scale-operator/pkg/3scale/amp/component/highavailability_options.go

Line 6 in ee2778a

BackendRedisQueuesEndpoint string `validate:"required"`

I like your approach and has a good trade-off of simplicity and valid fix. The proper fix should be removing highavailability_options_provider and have three options providers and only call to the required ones. But we can leave that for later implementation.

[qlty-cloud-legacy]

Code Climate has analyzed commit 0b3e1ed and detected 0 issues on this pull request.

View more on Code Climate.