diff --git a/Dockerfile b/Dockerfile index 8e772c85..7e04bec0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -46,7 +46,6 @@ FROM registry.access.redhat.com/ubi8/ubi-micro:latest # copy the service COPY --from=builder /opt/app-root/src/config.toml /config.toml COPY --from=builder /opt/app-root/src/insights-operator-gathering-conditions-service . -COPY --from=builder /opt/app-root/src/cluster-mapping.json . COPY --from=builder /opt/app-root/src/openapi.json . # copy the certificates @@ -56,6 +55,7 @@ COPY --from=builder /etc/pki /etc/pki # copy the conditions COPY --from=conditions /conditions /conditions COPY --from=conditions /remote-configurations /remote-configurations +COPY --from=conditions /mapping /mapping USER 1001 diff --git a/README.md b/README.md index 08630b34..1f89472c 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ Service provides option of canary rollout for new version of configurations. The ## Configure -Configuration is done by `toml` config, taking the `config/config.toml` in the working directory if no other configuration is provided. This can be overriden by `INSIGHTS_OPERATOR_CONDITIONAL_SERVICE_CONFIG_FILE` environment variable. +Configuration is done by `toml` config, taking the `config.toml` in the working directory if no other configuration is provided. This can be overriden by `INSIGHTS_OPERATOR_CONDITIONAL_SERVICE_CONFIG_FILE` environment variable. You can also override specific configuration options by setting specific environment variables in this form: @@ -96,17 +96,19 @@ For example, for a configuration like this ``` [storage] -remote_configuration = "./conditions/v2" -cluster_mapping = "cluster-mapping.json" +remote_configuration = "./conditions" +cluster_mapping = "./mapping" ``` you would override these values as follows: ``` export INSIGHTS_OPERATOR_GATHERING_CONDITIONS_SERVICE__STORAGE__REMOTE_CONFIGURATION=tests/rapid-recommendations -export INSIGHTS_OPERATOR_GATHERING_CONDITIONS_SERVICE__STORAGE__CLUSTER_MAPPING=tests/rapid-recommendations/cluster-mapping.json +export INSIGHTS_OPERATOR_GATHERING_CONDITIONS_SERVICE__STORAGE__CLUSTER_MAPPING=tests/mapping/ ``` +`remote_configuration`, `conditions` and `cluster_mapping` configuration options describe locations of respective content served by the service. However, each of these also contain separate `stable` and `canary` subdirectories containing different versions of the content. `cluster_mapping_file` option then describes the name of the file within `${cluster_mapping}/stable` and`${cluster_mapping}/canary`, and this file maps different OCP versions to the specific content under `${remote_configuration}/stable` (or `${remote_configuration}/canary`). + ## Conditions This service exposes the conditions from the @@ -151,7 +153,7 @@ There are some flags for different purposes: As part of [CCXDEV-12849](https://issues.redhat.com/browse/CCXDEV-12849) we introduced a new feature to map remote configurations to different OCP versions. -In order to use it you need to set the `cluster_mapping` and +In order to use it you need to set the `cluster_mapping`, `cluster_mapping_file` and `remote_configuration` fields in [config.toml] or use environment variables. The cluster map should look like this: ``` diff --git a/cluster-mapping.json b/cluster-mapping.json deleted file mode 100644 index 26e6494b..00000000 --- a/cluster-mapping.json +++ /dev/null @@ -1,3 +0,0 @@ -[ - ["4.0.0", "config_default.json"] -] \ No newline at end of file diff --git a/config.toml b/config.toml index 8e454e95..a31e5b6f 100644 --- a/config.toml +++ b/config.toml @@ -10,7 +10,8 @@ type = "jwt" [storage] rules_path = "./conditions" remote_configuration = "./remote-configurations" -cluster_mapping = "cluster-mapping.json" +cluster_mapping = "./mapping" +cluster_mapping_file = "cluster-mapping.json" [canary] unleash_enabled = false diff --git a/get_conditions.sh b/get_conditions.sh index 894de984..ee94745f 100755 --- a/get_conditions.sh +++ b/get_conditions.sh @@ -1,22 +1,26 @@ #!/bin/bash -STABLE_VERSION=1.1.0 -CANARY_VERSION=1.1.0 +STABLE_VERSION=1.1.2 +CANARY_VERSION=1.1.2 # Clone the conditions repo and build it to gather the conditions if [ ! -d 'insights-operator-gathering-conditions' ]; then git clone https://github.com/RedHatInsights/insights-operator-gathering-conditions; fi mkdir -p conditions mkdir -p remote-configurations +mkdir -p mapping/stable +mkdir -p mapping/canary cd insights-operator-gathering-conditions || exit 1 git checkout ${STABLE_VERSION} && \ ./build.sh && \ cp -r build/v1 ../conditions/stable && \ cp -r build/v2 ../remote-configurations/stable && \ +cp build/cluster-mapping.json ../mapping/stable/ && \ rm -r build ; \ git checkout ${CANARY_VERSION} && \ ./build.sh && \ cp -r build/v1 ../conditions/canary && \ cp -r build/v2 ../remote-configurations/canary && \ +cp build/cluster-mapping.json ../mapping/canary/ && \ rm -r build ; \ diff --git a/internal/service/cluster_mapping.go b/internal/service/cluster_mapping.go index 0151b2b2..3f129bdd 100644 --- a/internal/service/cluster_mapping.go +++ b/internal/service/cluster_mapping.go @@ -12,19 +12,29 @@ import ( ) // ClusterMapping map OCP versions to remote configurations -type ClusterMapping [][]string +type ClusterMapping struct { + version string + mapping [][]string +} + +func NewClusterMapping(version string, mapping [][]string) *ClusterMapping { + return &ClusterMapping{ + version: version, + mapping: mapping, + } +} // IsValid check the list is in order (based on the versions), that the versions // can be parsed and that the remote configurations are accessible -func (cm ClusterMapping) IsValid(remoteConfigurationPath string, conditionsVersion string) bool { +func (cm ClusterMapping) IsValid(remoteConfigurationPath string) bool { versions := []semver.Version{} // used to check if it's sorted - if len(cm) == 0 { - log.Error().Interface("raw", cm).Msg("Cluster map needs to contain at least one pair of version and filepath") + if len(cm.mapping) == 0 { + log.Error().Interface("raw", cm.mapping).Msg("Cluster map needs to contain at least one pair of version and filepath") return false } - for _, slice := range cm { + for _, slice := range cm.mapping { if len(slice) != 2 { log.Error().Int("len", len(slice)).Strs("slice", slice).Msg("Unexpected slice length") return false @@ -38,7 +48,7 @@ func (cm ClusterMapping) IsValid(remoteConfigurationPath string, conditionsVersi versions = append(versions, versionParsed) filepath := slice[1] - fullFilepath := fmt.Sprintf("%s/%s/%s", remoteConfigurationPath, conditionsVersion, filepath) + fullFilepath := fmt.Sprintf("%s/%s/%s", remoteConfigurationPath, cm.version, filepath) if _, err := os.Stat(fullFilepath); errors.Is(err, os.ErrNotExist) { log.Error().Str("filepath", fullFilepath). Msg("Remote configuration filepath couldn't be accessed") @@ -72,7 +82,7 @@ func (cm ClusterMapping) IsValid(remoteConfigurationPath string, conditionsVersi // than 3.0.0 func (cm ClusterMapping) GetFilepathForVersion(ocpVersionParsed semver.Version) (string, error) { // check the version is not greater than the first slice - firstVersion, err := semver.Make(cm[0][0]) + firstVersion, err := semver.Make(cm.mapping[0][0]) if err != nil { log.Error().Str("version", firstVersion.String()).Err(err).Msg("Invalid semver") return "", err @@ -88,11 +98,11 @@ func (cm ClusterMapping) GetFilepathForVersion(ocpVersionParsed semver.Version) return "", &merrors.NotFoundError{ ErrString: errMsg} } else if comparison == 0 { - return cm[0][1], nil + return cm.mapping[0][1], nil } - previousFilepath := cm[0][1] - for _, slice := range cm[1:] { + previousFilepath := cm.mapping[0][1] + for _, slice := range cm.mapping[1:] { version := slice[0] filepath := slice[1] versionParsed, err := semver.Make(version) @@ -115,5 +125,5 @@ func (cm ClusterMapping) GetFilepathForVersion(ocpVersionParsed semver.Version) log.Debug().Str("ocpVersion", ocpVersionParsed.String()). Msg("Returning latest remote configuration") - return cm[len(cm)-1][1], nil + return cm.mapping[len(cm.mapping)-1][1], nil } diff --git a/internal/service/cluster_mapping_test.go b/internal/service/cluster_mapping_test.go index da10c6b4..d7923c1f 100644 --- a/internal/service/cluster_mapping_test.go +++ b/internal/service/cluster_mapping_test.go @@ -13,53 +13,58 @@ const testFilesPath = "../../tests/rapid-recommendations" func TestClusterMappingIsValid(t *testing.T) { t.Run("valid map", func(t *testing.T) { - var sut service.ClusterMapping = [][]string{ + mapping := [][]string{ {"1.0.0", "experimental_1.json"}, {"2.0.0", "experimental_2.json"}, {"3.0.0", "config_default.json"}, } - assert.True(t, sut.IsValid(testFilesPath, service.StableVersion)) + sut := service.NewClusterMapping(service.StableVersion, mapping) + assert.True(t, sut.IsValid(testFilesPath)) }) t.Run("invalid map: invalid version", func(t *testing.T) { - var sut service.ClusterMapping = [][]string{ + mapping := [][]string{ {"1.0.0", "experimental_1.json"}, {"not a valid version", "experimental_2.json"}, {"3.0.0", "config_default.json"}, } - assert.False(t, sut.IsValid(testFilesPath, service.StableVersion)) + sut := service.NewClusterMapping(service.StableVersion, mapping) + assert.False(t, sut.IsValid(testFilesPath)) }) t.Run("invalid map: JSON not found", func(t *testing.T) { - var sut service.ClusterMapping = [][]string{ + mapping := [][]string{ {"1.0.0", "experimental_1.json"}, {"2.0.0", "not-found.json"}, {"3.0.0", "config_default.json"}, } - assert.False(t, sut.IsValid(testFilesPath, service.StableVersion)) + sut := service.NewClusterMapping(service.StableVersion, mapping) + assert.False(t, sut.IsValid(testFilesPath)) }) t.Run("invalid map: versions unsorted", func(t *testing.T) { - var sut service.ClusterMapping = [][]string{ + mapping := [][]string{ {"1.0.0", "experimental_1.json"}, {"3.0.0", "experimental_2.json"}, {"2.0.0", "config_default.json"}, } - assert.False(t, sut.IsValid(testFilesPath, service.StableVersion)) + sut := service.NewClusterMapping(service.StableVersion, mapping) + assert.False(t, sut.IsValid(testFilesPath)) }) t.Run("invalid map: map contains a tripple instead of key-values pairs", func(t *testing.T) { - var sut service.ClusterMapping = [][]string{ + mapping := [][]string{ {"1.0.0", "experimental_1.json", "some-element"}, {"2.0.0", "experimental_2.json"}, {"3.0.0", "config_default.json"}, } - assert.False(t, sut.IsValid(testFilesPath, service.StableVersion)) + sut := service.NewClusterMapping(service.StableVersion, mapping) + assert.False(t, sut.IsValid(testFilesPath)) }) t.Run("invalid map: map is empty", func(t *testing.T) { - var sut service.ClusterMapping = [][]string{} - assert.False(t, sut.IsValid(testFilesPath, service.StableVersion)) + sut := service.NewClusterMapping(service.StableVersion, [][]string{}) + assert.False(t, sut.IsValid(testFilesPath)) }) } @@ -102,11 +107,12 @@ func TestClusterMappingGetFilepathForVersion(t *testing.T) { }, } - var sut service.ClusterMapping = [][]string{ + mapping := [][]string{ {"1.0.0", "experimental_1.json"}, {"2.0.0", "experimental_2.json"}, {"3.0.0", "config_default.json"}, } + sut := service.NewClusterMapping(service.StableVersion, mapping) for _, tc := range testCases { t.Run(tc.ocpVersion, func(t *testing.T) { diff --git a/internal/service/common_test.go b/internal/service/common_test.go index 1f51e514..715b2557 100644 --- a/internal/service/common_test.go +++ b/internal/service/common_test.go @@ -145,14 +145,18 @@ type mockStorage struct { getRemoteConfigurationFilepathMockError error } -func (m *mockStorage) ReadConditionalRules(*http.Request, string) []byte { +func (m *mockStorage) IsCanary(*http.Request) bool { + return true +} + +func (m *mockStorage) ReadConditionalRules(bool, string) []byte { return m.conditionalRules } -func (m *mockStorage) ReadRemoteConfig(*http.Request, string) []byte { +func (m *mockStorage) ReadRemoteConfig(bool, string) []byte { return m.remoteConfig } -func (m *mockStorage) GetRemoteConfigurationFilepath(string) (string, error) { +func (m *mockStorage) GetRemoteConfigurationFilepath(bool, string) (string, error) { return m.remoteConfigFilepath, m.getRemoteConfigurationFilepathMockError } diff --git a/internal/service/repository.go b/internal/service/repository.go index 545e90b0..72a45384 100644 --- a/internal/service/repository.go +++ b/internal/service/repository.go @@ -70,7 +70,7 @@ func NewRepository(s StorageInterface) *Repository { // Rules method reads all and unmarshals all rules stored under given path func (r *Repository) Rules(request *http.Request) (*Rules, error) { filepath := "rules.json" // TODO: Make this configurable - data := r.store.ReadConditionalRules(request, filepath) + data := r.store.ReadConditionalRules(r.store.IsCanary(request), filepath) if data == nil { return nil, fmt.Errorf("store data not found for '%s'", filepath) } @@ -87,11 +87,12 @@ func (r *Repository) Rules(request *http.Request) (*Rules, error) { // RemoteConfiguration returns a remote configuration for v2 endpoint based on // the cluster map defined in the settings and loaded on startup func (r *Repository) RemoteConfiguration(request *http.Request, ocpVersion string) (*RemoteConfiguration, error) { - filepath, err := r.store.GetRemoteConfigurationFilepath(ocpVersion) + isCanary := r.store.IsCanary(request) + filepath, err := r.store.GetRemoteConfigurationFilepath(isCanary, ocpVersion) if err != nil { return nil, err } - data := r.store.ReadRemoteConfig(request, filepath) + data := r.store.ReadRemoteConfig(isCanary, filepath) if data == nil { return nil, fmt.Errorf("store data not found for '%s'", filepath) } diff --git a/internal/service/service_test.go b/internal/service/service_test.go index cf6ecf39..0fde3915 100644 --- a/internal/service/service_test.go +++ b/internal/service/service_test.go @@ -155,128 +155,129 @@ func TestServiceV2(t *testing.T) { func TestServiceV2WithClusterMapping(t *testing.T) { type testCase struct { - name string - clusterMappingFilepath string - expectedAnError bool - ocpVersion string - wantConfiguration string - expect400 bool - expect404 bool + name string + clusterMappingFile string + expectedAnError bool + ocpVersion string + wantConfiguration string + expect400 bool + expect404 bool } const ( - validClusterMapping = "../../tests/rapid-recommendations/cluster-mapping.json" - malformedClusterMapping = "../../tests/rapid-recommendations/malformed-cluster-mapping.json" - notFoundClusterMapping = "../../tests/rapid-recommendations/not-found-cluster-mapping.json" + clusterMappingPath = "../../tests/mapping/" + validClusterMapping = "cluster-mapping.json" + malformedClusterMapping = "malformed-cluster-mapping.json" + notFoundClusterMapping = "not-found-cluster-mapping.json" internalServerError = `{"status":"Internal Server Error"}` ) testCases := []testCase{ { - name: "invalid cluster mapping", - clusterMappingFilepath: malformedClusterMapping, - expectedAnError: true, - ocpVersion: "any version", - wantConfiguration: "", + name: "invalid cluster mapping", + clusterMappingFile: malformedClusterMapping, + expectedAnError: true, + ocpVersion: "any version", + wantConfiguration: "", }, { - name: "cluster mapping not found", - clusterMappingFilepath: notFoundClusterMapping, - expectedAnError: true, - ocpVersion: "", - wantConfiguration: "", + name: "cluster mapping not found", + clusterMappingFile: notFoundClusterMapping, + expectedAnError: true, + ocpVersion: "", + wantConfiguration: "", }, { - name: "cluster mapping is undefined", - clusterMappingFilepath: "", - expectedAnError: true, - ocpVersion: "", - wantConfiguration: "", + name: "cluster mapping is undefined", + clusterMappingFile: "", + expectedAnError: true, + ocpVersion: "", + wantConfiguration: "", }, { - name: "valid cluster mapping and version out of range", - clusterMappingFilepath: validClusterMapping, - expectedAnError: false, - ocpVersion: "any version", - wantConfiguration: configDefaultConfiguration, - expect400: true, + name: "valid cluster mapping and version out of range", + clusterMappingFile: validClusterMapping, + expectedAnError: false, + ocpVersion: "any version", + wantConfiguration: configDefaultConfiguration, + expect400: true, }, { - name: "cluster version prior to 4.0", - clusterMappingFilepath: validClusterMapping, - expectedAnError: false, - ocpVersion: "1.2.3", - expect404: true, + name: "cluster version prior to 4.0", + clusterMappingFile: validClusterMapping, + expectedAnError: false, + ocpVersion: "1.2.3", + expect404: true, }, { - name: "cluster version is 4.0.0", - clusterMappingFilepath: validClusterMapping, - expectedAnError: false, - ocpVersion: "4.0.0", - wantConfiguration: emptyConfiguration, + name: "cluster version is 4.0.0", + clusterMappingFile: validClusterMapping, + expectedAnError: false, + ocpVersion: "4.0.0", + wantConfiguration: emptyConfiguration, }, { - name: "cluster version is between 4.0.0 and 4.17.0-0", - clusterMappingFilepath: validClusterMapping, - expectedAnError: false, - ocpVersion: "4.5.6", - wantConfiguration: emptyConfiguration, + name: "cluster version is between 4.0.0 and 4.17.0-0", + clusterMappingFile: validClusterMapping, + expectedAnError: false, + ocpVersion: "4.5.6", + wantConfiguration: emptyConfiguration, }, { - name: "cluster version is 4.17.0-0", - clusterMappingFilepath: validClusterMapping, - expectedAnError: false, - ocpVersion: "4.17.0-0", - wantConfiguration: experimental1Configuration, + name: "cluster version is 4.17.0-0", + clusterMappingFile: validClusterMapping, + expectedAnError: false, + ocpVersion: "4.17.0-0", + wantConfiguration: experimental1Configuration, }, { - name: "cluster version is between 4.17.0-0 and 4.17.0", - clusterMappingFilepath: validClusterMapping, - expectedAnError: false, - ocpVersion: "4.17.0-5", - wantConfiguration: experimental1Configuration, + name: "cluster version is between 4.17.0-0 and 4.17.0", + clusterMappingFile: validClusterMapping, + expectedAnError: false, + ocpVersion: "4.17.0-5", + wantConfiguration: experimental1Configuration, }, { - name: "cluster version is between 4.17.0 and 4.17.5", - clusterMappingFilepath: validClusterMapping, - expectedAnError: false, - ocpVersion: "4.17.3", - wantConfiguration: experimental2Configuration, + name: "cluster version is between 4.17.0 and 4.17.5", + clusterMappingFile: validClusterMapping, + expectedAnError: false, + ocpVersion: "4.17.3", + wantConfiguration: experimental2Configuration, }, { - name: "cluster version is between 4.17.5 and 4.17.6", - clusterMappingFilepath: validClusterMapping, - expectedAnError: false, - ocpVersion: "4.17.6-alpha", - wantConfiguration: bugWorkaroundConfiguration, + name: "cluster version is between 4.17.5 and 4.17.6", + clusterMappingFile: validClusterMapping, + expectedAnError: false, + ocpVersion: "4.17.6-alpha", + wantConfiguration: bugWorkaroundConfiguration, }, { - name: "cluster version is a CI release of 4.17.6", - clusterMappingFilepath: validClusterMapping, - expectedAnError: false, - ocpVersion: "4.17.6-0.ci-2024-08-19-220527", - wantConfiguration: bugWorkaroundConfiguration, + name: "cluster version is a CI release of 4.17.6", + clusterMappingFile: validClusterMapping, + expectedAnError: false, + ocpVersion: "4.17.6-0.ci-2024-08-19-220527", + wantConfiguration: bugWorkaroundConfiguration, }, { - name: "cluster version is a release candidate of 4.17.6", - clusterMappingFilepath: validClusterMapping, - expectedAnError: false, - ocpVersion: "4.17.6-rc.0", - wantConfiguration: bugWorkaroundConfiguration, + name: "cluster version is a release candidate of 4.17.6", + clusterMappingFile: validClusterMapping, + expectedAnError: false, + ocpVersion: "4.17.6-rc.0", + wantConfiguration: bugWorkaroundConfiguration, }, { - name: "cluster version is a prerelease of 4.17.6", - clusterMappingFilepath: validClusterMapping, - expectedAnError: false, - ocpVersion: "4.17.6-alpha", - wantConfiguration: bugWorkaroundConfiguration, + name: "cluster version is a prerelease of 4.17.6", + clusterMappingFile: validClusterMapping, + expectedAnError: false, + ocpVersion: "4.17.6-alpha", + wantConfiguration: bugWorkaroundConfiguration, }, { - name: "cluster version greater than 4.17.6", - clusterMappingFilepath: validClusterMapping, - expectedAnError: false, - ocpVersion: "5.6.7", - wantConfiguration: experimental2Configuration, + name: "cluster version greater than 4.17.6", + clusterMappingFile: validClusterMapping, + expectedAnError: false, + ocpVersion: "5.6.7", + wantConfiguration: experimental2Configuration, }, } @@ -285,7 +286,8 @@ func TestServiceV2WithClusterMapping(t *testing.T) { store, err := service.NewStorage(service.StorageConfig{ RulesPath: "../../tests/conditions", RemoteConfigurationPath: "../../tests/rapid-recommendations", - ClusterMappingPath: tc.clusterMappingFilepath, + ClusterMappingPath: clusterMappingPath, + ClusterMappingFile: tc.clusterMappingFile, }, true, &MockUnleashClient{}) if tc.expectedAnError { assert.Error(t, err, "this configuration should have made the service crash") diff --git a/internal/service/storage.go b/internal/service/storage.go index 54095c81..b440a903 100644 --- a/internal/service/storage.go +++ b/internal/service/storage.go @@ -50,9 +50,10 @@ type UnleashClientInterface interface { // StorageInterface describe interface to be implemented by resource storage // implementations. type StorageInterface interface { - ReadConditionalRules(request *http.Request, res string) []byte - ReadRemoteConfig(request *http.Request, p string) []byte - GetRemoteConfigurationFilepath(ocpVersion string) (string, error) + IsCanary(request *http.Request) bool + ReadConditionalRules(isCanary bool, res string) []byte + ReadRemoteConfig(isCanary bool, p string) []byte + GetRemoteConfigurationFilepath(isCanary bool, ocpVersion string) (string, error) } // StorageConfig structure contains configuration for resource storage. @@ -60,6 +61,7 @@ type StorageConfig struct { RulesPath string `mapstructure:"rules_path" toml:"rules_path"` RemoteConfigurationPath string `mapstructure:"remote_configuration" toml:"remote_configuration"` ClusterMappingPath string `mapstructure:"cluster_mapping" toml:"cluster_mapping"` + ClusterMappingFile string `mapstructure:"cluster_mapping_file" toml:"cluster_mapping_file"` } // CanaryConfig structure contains configuration for canary rollout @@ -123,7 +125,9 @@ type Storage struct { remoteConfigurationPath string cache Cache clusterMappingPath string - clusterMapping ClusterMapping + clusterMappingFile string + stableClusterMapping *ClusterMapping + canaryClusterMapping *ClusterMapping unleashClient UnleashClientInterface unleashEnabled bool } @@ -135,78 +139,103 @@ func NewStorage(storageConfig StorageConfig, unleashEnabled bool, unleashClient conditionalRulesPath: storageConfig.RulesPath, remoteConfigurationPath: storageConfig.RemoteConfigurationPath, clusterMappingPath: storageConfig.ClusterMappingPath, + clusterMappingFile: storageConfig.ClusterMappingFile, unleashEnabled: unleashEnabled, unleashClient: unleashClient, } + cm, err := s.loadClusterMapping(StableVersion) + if err != nil { + log.Error().Err(err).Msg("Could not load stable version of cluster mapping") + return &s, err + } + s.stableClusterMapping = cm + + cm, err = s.loadClusterMapping(CanaryVersion) + if err != nil { + log.Error().Err(err).Msg("Could not load canary version of cluster mapping") + return &s, err + } + s.canaryClusterMapping = cm + + return &s, nil +} + +func (s *Storage) loadClusterMapping(version string) (*ClusterMapping, error) { if s.clusterMappingPath == "" { - errStr := "cluster mapping filepath is not defined" + errStr := "cluster mapping directory path is not defined" log.Error().Msg(errStr) - return &s, errors.New(errStr) + return nil, errors.New(errStr) } + + if s.clusterMappingFile == "" { + errStr := "cluster mapping file name is not defined" + log.Error().Msg(errStr) + return nil, errors.New(errStr) + } + // Parse the cluster map - cm := ClusterMapping{} - rawData := s.readDataFromPath(s.clusterMappingPath) + cm := ClusterMapping{ + version: version, + mapping: [][]string{}, + } + fullFilepath := fmt.Sprintf("%s/%s/%s", s.clusterMappingPath, version, s.clusterMappingFile) + log.Info().Msg(fullFilepath) + rawData := s.readDataFromPath(fullFilepath) if rawData == nil { - return &s, errors.New("cannot find cluster map") + return nil, errors.New("cannot find cluster map") } - err := json.Unmarshal(rawData, &cm) + err := json.Unmarshal(rawData, &cm.mapping) if err != nil { - log.Error().Err(err).Msg("Cannot load cluster map") - return &s, err + log.Error().Str("version", version).Err(err).Msg("Cannot load cluster map") + return nil, err } - log.Debug().Interface("cluster-map", cm).Msg("Cluster map loaded") + log.Debug().Interface("cluster-map", cm.mapping).Msg("Cluster map loaded") - if cm.IsValid(s.remoteConfigurationPath, StableVersion) { - log.Info().Msg("The stable version of cluster map JSON is valid") - s.clusterMapping = cm + if cm.IsValid(s.remoteConfigurationPath) { + log.Info().Str("version", version).Msg("The cluster map JSON is valid") } else { - log.Error().Msg("Stable version of cluster map is invalid") + log.Error().Str("version", version).Msg("The cluster map is invalid") return nil, errors.New("cannot parse cluster map") } - if cm.IsValid(s.remoteConfigurationPath, CanaryVersion) { - log.Info().Msg("The canary version of cluster map JSON is valid") + return &cm, nil +} + +// IsCanary queries UnleashClient to determine which version of configurations to serve +func (s *Storage) IsCanary(r *http.Request) bool { + if !s.unleashEnabled { + return false + } + // We use User-Agent header to decide between stable and canary version (header contains cluster ID) + clusterID := GetClusterID(r) + isCanary := s.unleashClient.IsCanary(clusterID) + if isCanary { + log.Debug().Str("canary argument", clusterID).Msg("Serving canary version of configurations") } else { - log.Error().Msg("Canary version of cluster map is invalid") - return nil, errors.New("cannot parse cluster map") + log.Debug().Str("canary argument", clusterID).Msg("Serving stable version of configurations") } - - return &s, nil + return isCanary } // ReadConditionalRules tries to find conditional rule with given name in the storage. -func (s *Storage) ReadConditionalRules(r *http.Request, path string) []byte { +func (s *Storage) ReadConditionalRules(isCanary bool, path string) []byte { log.Debug().Str("path to resource", path).Msg("Finding resource") version := StableVersion - clusterID := GetClusterID(r) - if s.unleashEnabled { - // We use User-Agent header to decide between stable and canary version (header contains cluster ID) - if s.unleashClient.IsCanary(clusterID) { - log.Debug().Str("canary argument", clusterID).Msg("Served canary version of rules") - version = CanaryVersion - } else { - log.Debug().Str("canary argument", clusterID).Msg("Served stable version of rules") - } + if isCanary { + version = CanaryVersion } conditionalRulesPath := fmt.Sprintf("%s/%s/%s", s.conditionalRulesPath, version, path) return s.readDataFromPath(conditionalRulesPath) } // ReadRemoteConfig tries to find remote configuration with given name in the storage -func (s *Storage) ReadRemoteConfig(r *http.Request, path string) []byte { +func (s *Storage) ReadRemoteConfig(isCanary bool, path string) []byte { log.Debug().Str("path to resource", path).Msg("Finding resource") version := StableVersion - clusterID := GetClusterID(r) - if s.unleashEnabled { - // We use User-Agent header to decide between stable and canary version (header contains cluster ID) - if s.unleashClient.IsCanary(clusterID) { - log.Debug().Str("canary argument", clusterID).Msg("Served canary version of remote configurations") - version = CanaryVersion - } else { - log.Debug().Str("canary argument", clusterID).Msg("Served stable version of remote configurations") - } + if isCanary { + version = CanaryVersion } remoteConfigPath := fmt.Sprintf("%s/%s/%s", s.remoteConfigurationPath, version, path) return s.readDataFromPath(remoteConfigPath) @@ -214,7 +243,7 @@ func (s *Storage) ReadRemoteConfig(r *http.Request, path string) []byte { // GetRemoteConfigurationFilepath returns the filepath to the remote configuration // that should be returned for the given OCP version based on the cluster map -func (s *Storage) GetRemoteConfigurationFilepath(ocpVersion string) (string, error) { +func (s *Storage) GetRemoteConfigurationFilepath(isCanary bool, ocpVersion string) (string, error) { ocpVersionParsed, err := semver.Make(ocpVersion) if err != nil { log.Error().Str("ocpVersion", ocpVersion).Err(err).Msg("Invalid semver") @@ -224,7 +253,10 @@ func (s *Storage) GetRemoteConfigurationFilepath(ocpVersion string) (string, err ErrString: err.Error()} } - return s.clusterMapping.GetFilepathForVersion(ocpVersionParsed) + if isCanary { + return s.canaryClusterMapping.GetFilepathForVersion(ocpVersionParsed) + } + return s.stableClusterMapping.GetFilepathForVersion(ocpVersionParsed) } func (s *Storage) readDataFromPath(path string) []byte { diff --git a/internal/service/storage_test.go b/internal/service/storage_test.go index 6d85eaf3..d0030fa8 100644 --- a/internal/service/storage_test.go +++ b/internal/service/storage_test.go @@ -26,10 +26,11 @@ import ( ) const ( - validRulesFile = "rules.json" - rulesFolder = "testdata/v1" - v2Folder = "testdata/v2" - clusterMappingFilepath = "testdata/cluster-mapping.json" + validRulesFile = "rules.json" + rulesFolder = "testdata/v1" + v2Folder = "testdata/v2" + clusterMappingPath = "testdata/mapping" + clusterMappingFile = "cluster-mapping.json" ) type MockUnleashClient struct{} @@ -51,7 +52,8 @@ func TestNewStorage(t *testing.T) { config: service.StorageConfig{ RulesPath: "testdata/v1", RemoteConfigurationPath: "testdata/v2", - ClusterMappingPath: "testdata/cluster-mapping.json", + ClusterMappingPath: "testdata/mapping", + ClusterMappingFile: "cluster-mapping.json", }, expectError: false, }, @@ -60,7 +62,8 @@ func TestNewStorage(t *testing.T) { config: service.StorageConfig{ RulesPath: "testdata/v1", RemoteConfigurationPath: "testdata/v2_missing_stable", - ClusterMappingPath: "testdata/cluster-mapping.json", + ClusterMappingPath: "testdata/mapping", + ClusterMappingFile: "cluster-mapping.json", }, expectError: true, }, @@ -69,7 +72,8 @@ func TestNewStorage(t *testing.T) { config: service.StorageConfig{ RulesPath: "testdata/v1", RemoteConfigurationPath: "testdata/v2_missing_canary", - ClusterMappingPath: "testdata/cluster-mapping.json", + ClusterMappingPath: "testdata/mapping", + ClusterMappingFile: "cluster-mapping.json", }, expectError: true, }, @@ -122,7 +126,8 @@ func TestReadConditionalRules(t *testing.T) { storage, err := service.NewStorage( service.StorageConfig{ RulesPath: rulesFolder, - ClusterMappingPath: clusterMappingFilepath, + ClusterMappingPath: clusterMappingPath, + ClusterMappingFile: clusterMappingFile, RemoteConfigurationPath: v2Folder, }, false, nil) assert.NoError(t, err) @@ -135,7 +140,8 @@ func TestReadConditionalRules(t *testing.T) { storage, err := service.NewStorage( service.StorageConfig{ RulesPath: rulesFolder, - ClusterMappingPath: clusterMappingFilepath, + ClusterMappingPath: clusterMappingPath, + ClusterMappingFile: clusterMappingFile, RemoteConfigurationPath: v2Folder, }, false, nil) assert.NoError(t, err) @@ -170,7 +176,8 @@ func TestReadRulesCanaryRollout(t *testing.T) { storage, err := service.NewStorage( service.StorageConfig{ RulesPath: rulesFolder, - ClusterMappingPath: clusterMappingFilepath, + ClusterMappingPath: clusterMappingPath, + ClusterMappingFile: clusterMappingFile, RemoteConfigurationPath: v2Folder, }, true, &MockUnleashClient{}) assert.NoError(t, err) @@ -184,7 +191,7 @@ func TestReadRulesCanaryRollout(t *testing.T) { func checkConditionalRules(t *testing.T, storage *service.Storage, rulesFile string, expectedRules service.Rules, r *http.Request) { var rules service.Rules - data := storage.ReadConditionalRules(r, rulesFile) + data := storage.ReadConditionalRules(storage.IsCanary(r), rulesFile) if len(data) == 0 { rules = service.Rules{} } else { @@ -196,7 +203,7 @@ func checkConditionalRules(t *testing.T, storage *service.Storage, rulesFile str func checkRemoteConfig(t *testing.T, storage *service.Storage, remoteConfigFile string, expectedRemoteConfig service.RemoteConfiguration, r *http.Request) { var remoteConfig service.RemoteConfiguration - data := storage.ReadRemoteConfig(r, remoteConfigFile) + data := storage.ReadRemoteConfig(storage.IsCanary(r), remoteConfigFile) if len(data) == 0 { remoteConfig = service.RemoteConfiguration{} } else { @@ -239,7 +246,8 @@ func TestReadRemoteConfiguration(t *testing.T) { storage, err := service.NewStorage( service.StorageConfig{ RemoteConfigurationPath: v2Folder, - ClusterMappingPath: clusterMappingFilepath, + ClusterMappingPath: clusterMappingPath, + ClusterMappingFile: clusterMappingFile, }, false, nil) assert.NoError(t, err) checkRemoteConfig(t, storage, tt.remoteConfigFile, tt.expectedRemoteConfig, &http.Request{}) @@ -272,7 +280,8 @@ func TestReadRemoteConfigurationCanaryRollout(t *testing.T) { storage, err := service.NewStorage( service.StorageConfig{ RemoteConfigurationPath: v2Folder, - ClusterMappingPath: clusterMappingFilepath, + ClusterMappingPath: clusterMappingPath, + ClusterMappingFile: clusterMappingFile, }, true, &MockUnleashClient{}) assert.NoError(t, err) req, err := http.NewRequest("GET", "http://example.com", nil) diff --git a/internal/service/testdata/cluster-mapping.json b/internal/service/testdata/mapping/canary/cluster-mapping.json similarity index 100% rename from internal/service/testdata/cluster-mapping.json rename to internal/service/testdata/mapping/canary/cluster-mapping.json diff --git a/internal/service/testdata/mapping/stable/cluster-mapping.json b/internal/service/testdata/mapping/stable/cluster-mapping.json new file mode 100644 index 00000000..91dce8d4 --- /dev/null +++ b/internal/service/testdata/mapping/stable/cluster-mapping.json @@ -0,0 +1,3 @@ +[ + ["1.0.0", "rules.json"] +] \ No newline at end of file diff --git a/tests/config.toml b/tests/config.toml index 3e1c2b84..af14a978 100644 --- a/tests/config.toml +++ b/tests/config.toml @@ -6,7 +6,8 @@ enable_cors = false [storage] rules_path = "tests/conditions" remote_configuration = "./tests/rapid-recommendations" -cluster_mapping = "./tests/rapid-recommendations/cluster-mapping.json" +cluster_mapping = "./tests/mapping/" +cluster_mapping_file = "cluster-mapping.json" [canary] unleash_enabled = false diff --git a/tests/rapid-recommendations/cluster-mapping.json b/tests/mapping/canary/cluster-mapping.json similarity index 100% rename from tests/rapid-recommendations/cluster-mapping.json rename to tests/mapping/canary/cluster-mapping.json diff --git a/tests/mapping/stable/cluster-mapping.json b/tests/mapping/stable/cluster-mapping.json new file mode 100644 index 00000000..7c21f57c --- /dev/null +++ b/tests/mapping/stable/cluster-mapping.json @@ -0,0 +1,7 @@ +[ + ["4.0.0", "empty.json"], + ["4.17.0-0", "experimental_1.json"], + ["4.17.0", "experimental_2.json"], + ["4.17.5", "bug_workaround.json"], + ["4.17.6", "experimental_2.json"] +]