diff --git a/pkg/authorizer/clusterauthorizer/clusterauthorizer.go b/pkg/authorizer/clusterauthorizer/clusterauthorizer.go index 83c04bac8..2948fea33 100644 --- a/pkg/authorizer/clusterauthorizer/clusterauthorizer.go +++ b/pkg/authorizer/clusterauthorizer/clusterauthorizer.go @@ -13,14 +13,16 @@ import ( ) type Authorizer struct { - configurator configobserver.Configurator + secretConfigurator configobserver.Configurator + configurator configobserver.Interface // exposed for tests proxyFromEnvironment func(*http.Request) (*url.URL, error) } // New creates a new Authorizer, whose purpose is to auth requests for outgoing traffic. -func New(configurator configobserver.Configurator) *Authorizer { +func New(secretConfigurator configobserver.Configurator, configurator configobserver.Interface) *Authorizer { return &Authorizer{ + secretConfigurator: secretConfigurator, configurator: configurator, proxyFromEnvironment: http.ProxyFromEnvironment, } @@ -46,11 +48,11 @@ func (a *Authorizer) Authorize(req *http.Request) error { func (a *Authorizer) NewSystemOrConfiguredProxy() func(*http.Request) (*url.URL, error) { // using specific proxy settings if c := a.configurator.Config(); c != nil { - if len(c.HTTPConfig.HTTPProxy) > 0 || len(c.HTTPConfig.HTTPSProxy) > 0 || len(c.HTTPConfig.NoProxy) > 0 { + if len(c.Proxy.HTTPProxy) > 0 || len(c.Proxy.HTTPSProxy) > 0 || len(c.Proxy.NoProxy) > 0 { proxyConfig := httpproxy.Config{ - HTTPProxy: c.HTTPConfig.HTTPProxy, - HTTPSProxy: c.HTTPConfig.HTTPSProxy, - NoProxy: c.HTTPConfig.NoProxy, + HTTPProxy: c.Proxy.HTTPProxy, + HTTPSProxy: c.Proxy.HTTPSProxy, + NoProxy: c.Proxy.NoProxy, } // The golang ProxyFunc seems to have NoProxy already built in return func(req *http.Request) (*url.URL, error) { @@ -63,7 +65,7 @@ func (a *Authorizer) NewSystemOrConfiguredProxy() func(*http.Request) (*url.URL, } func (a *Authorizer) Token() (string, error) { - cfg := a.configurator.Config() + cfg := a.secretConfigurator.Config() if len(cfg.Token) > 0 { token := strings.TrimSpace(cfg.Token) if strings.Contains(token, "\n") || strings.Contains(token, "\r") { diff --git a/pkg/authorizer/clusterauthorizer/clusterauthorizer_test.go b/pkg/authorizer/clusterauthorizer/clusterauthorizer_test.go index be5322572..b2fc18ca6 100644 --- a/pkg/authorizer/clusterauthorizer/clusterauthorizer_test.go +++ b/pkg/authorizer/clusterauthorizer/clusterauthorizer_test.go @@ -22,11 +22,11 @@ func nonCachedProxyFromEnvironment() func(*http.Request) (*url.URL, error) { func Test_Proxy(tt *testing.T) { testCases := []struct { - Name string - EnvValues map[string]interface{} - RequestURL string - HTTPConfig config.HTTPConfig - ProxyURL string + Name string + EnvValues map[string]interface{} + RequestURL string + ProxyConfig config.Proxy + ProxyURL string }{ { Name: "No env set, no specific proxy", @@ -47,39 +47,39 @@ func Test_Proxy(tt *testing.T) { ProxyURL: "http://secproxy.to", }, { - Name: "Env not set, specific proxy set", - EnvValues: map[string]interface{}{"HTTP_PROXY": nil}, - RequestURL: "http://google.com", - HTTPConfig: config.HTTPConfig{HTTPProxy: "specproxy.to"}, - ProxyURL: "http://specproxy.to", + Name: "Env not set, specific proxy set", + EnvValues: map[string]interface{}{"HTTP_PROXY": nil}, + RequestURL: "http://google.com", + ProxyConfig: config.Proxy{HTTPProxy: "specproxy.to"}, + ProxyURL: "http://specproxy.to", }, { - Name: "Env set, specific proxy set http", - EnvValues: map[string]interface{}{"HTTP_PROXY": "envproxy.to"}, - RequestURL: "http://google.com", - HTTPConfig: config.HTTPConfig{HTTPProxy: "specproxy.to"}, - ProxyURL: "http://specproxy.to", + Name: "Env set, specific proxy set http", + EnvValues: map[string]interface{}{"HTTP_PROXY": "envproxy.to"}, + RequestURL: "http://google.com", + ProxyConfig: config.Proxy{HTTPProxy: "specproxy.to"}, + ProxyURL: "http://specproxy.to", }, { - Name: "Env set, specific proxy set https", - EnvValues: map[string]interface{}{"HTTPS_PROXY": "envsecproxy.to"}, - RequestURL: "https://google.com", - HTTPConfig: config.HTTPConfig{HTTPSProxy: "specsecproxy.to"}, - ProxyURL: "http://specsecproxy.to", + Name: "Env set, specific proxy set https", + EnvValues: map[string]interface{}{"HTTPS_PROXY": "envsecproxy.to"}, + RequestURL: "https://google.com", + ProxyConfig: config.Proxy{HTTPSProxy: "specsecproxy.to"}, + ProxyURL: "http://specsecproxy.to", }, { - Name: "Env set, specific proxy set noproxy, request without noproxy", - EnvValues: map[string]interface{}{"HTTPS_PROXY": "envsecproxy.to", "NO_PROXY": "envnoproxy.to"}, - RequestURL: "https://google.com", - HTTPConfig: config.HTTPConfig{HTTPSProxy: "specsecproxy.to", NoProxy: "specnoproxy.to"}, - ProxyURL: "http://specsecproxy.to", + Name: "Env set, specific proxy set noproxy, request without noproxy", + EnvValues: map[string]interface{}{"HTTPS_PROXY": "envsecproxy.to", "NO_PROXY": "envnoproxy.to"}, + RequestURL: "https://google.com", + ProxyConfig: config.Proxy{HTTPSProxy: "specsecproxy.to", NoProxy: "specnoproxy.to"}, + ProxyURL: "http://specsecproxy.to", }, { - Name: "Env set, specific proxy set noproxy, request with noproxy", - EnvValues: map[string]interface{}{"HTTPS_PROXY": "envsecproxy.to", "NO_PROXY": "envnoproxy.to"}, - RequestURL: "https://specnoproxy.to", - HTTPConfig: config.HTTPConfig{HTTPSProxy: "specsecproxy.to", NoProxy: "specnoproxy.to"}, - ProxyURL: "", + Name: "Env set, specific proxy set noproxy, request with noproxy", + EnvValues: map[string]interface{}{"HTTPS_PROXY": "envsecproxy.to", "NO_PROXY": "envnoproxy.to"}, + RequestURL: "https://specnoproxy.to", + ProxyConfig: config.Proxy{HTTPSProxy: "specsecproxy.to", NoProxy: "specnoproxy.to"}, + ProxyURL: "", }, } for _, tcase := range testCases { @@ -97,8 +97,15 @@ func Test_Proxy(tt *testing.T) { } } - co2 := &config.MockSecretConfigurator{Conf: &config.Controller{HTTPConfig: tc.HTTPConfig}} - a := Authorizer{proxyFromEnvironment: nonCachedProxyFromEnvironment(), configurator: co2} + secretConfigurator := &config.MockSecretConfigurator{Conf: &config.Controller{}} + configurator := config.NewMockConfigMapConfigurator(&config.InsightsConfiguration{ + Proxy: tc.ProxyConfig, + }) + a := Authorizer{ + proxyFromEnvironment: nonCachedProxyFromEnvironment(), + secretConfigurator: secretConfigurator, + configurator: configurator, + } p := a.NewSystemOrConfiguredProxy() req := httptest.NewRequest("GET", tc.RequestURL, http.NoBody) urlRec, err := p(req) diff --git a/pkg/config/config.go b/pkg/config/config.go index f54e1577d..2ab47ab55 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,329 +1,141 @@ package config import ( - "encoding/json" "fmt" + "strings" "time" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/klog/v2" ) -// Serialized defines the standard config for this operator. -type Serialized struct { - Report bool `json:"report"` - StoragePath string `json:"storagePath"` - Interval string `json:"interval"` - Endpoint string `json:"endpoint"` - ConditionalGathererEndpoint string `json:"conditionalGathererEndpoint"` - PullReport struct { - Endpoint string `json:"endpoint"` - Delay string `json:"delay"` - Timeout string `json:"timeout"` - MinRetryTime string `json:"min_retry"` - } `json:"pull_report"` - Impersonate string `json:"impersonate"` - EnableGlobalObfuscation bool `json:"enableGlobalObfuscation"` - OCM struct { - SCAEndpoint string `json:"scaEndpoint"` - SCAInterval string `json:"scaInterval"` - SCADisabled bool `json:"scaDisabled"` - ClusterTransferEndpoint string `json:"clusterTransferEndpoint"` - ClusterTransferInterval string `json:"clusterTransferInterval"` - } `json:"ocm"` - DisableInsightsAlerts bool `json:"disableInsightsAlerts"` - ProcessingStatusEndpoint string `json:"processingStatusEndpoint"` - ReportEndpointTechPreview string `json:"reportEndpointTechPreview"` -} - -// Controller defines the standard config for this operator. -type Controller struct { - Report bool - StoragePath string - Interval time.Duration - Endpoint string - ConditionalGathererEndpoint string - ReportEndpoint string - ReportEndpointTechPreview string - ReportPullingDelay time.Duration - ReportMinRetryTime time.Duration - ReportPullingTimeout time.Duration - Impersonate string - // EnableGlobalObfuscation enables obfuscation of domain names and IP addresses - // To see the detailed info about how anonymization works, go to the docs of package anonymization. - EnableGlobalObfuscation bool - - Token string - - HTTPConfig HTTPConfig - OCMConfig OCMConfig - - // DisableInsightsAlerts disabled exposing of Insights recommendations as Prometheus info alerts - DisableInsightsAlerts bool - ProcessingStatusEndpoint string -} - -// HTTPConfig configures http proxy and exception settings if they come from config -type HTTPConfig struct { - HTTPProxy string - HTTPSProxy string - NoProxy string -} - -// OCMConfig configures the interval and endpoint for retrieving the data from OCM API -type OCMConfig struct { - SCAInterval time.Duration - SCAEndpoint string - SCADisabled bool - ClusterTransferEndpoint string - ClusterTransferInterval time.Duration -} - -type Converter func(s *Serialized, cfg *Controller) (*Controller, error) - -// ToString returns the important fields of the config in a string form -func (c *Controller) ToString() string { - return fmt.Sprintf("enabled=%t "+ - "endpoint=%s "+ - "conditional_gatherer_endpoint=%s "+ - "interval=%s "+ - "token=%t "+ - "reportEndpoint=%s "+ - "initialPollingDelay=%s "+ - "minRetryTime=%s "+ - "pollingTimeout=%s "+ - "processingStatusEndpoint=%s", - c.Report, - c.Endpoint, - c.ConditionalGathererEndpoint, - c.Interval, - len(c.Token) > 0, - c.ReportEndpoint, - c.ReportPullingDelay, - c.ReportMinRetryTime, - c.ReportPullingTimeout, - c.ProcessingStatusEndpoint) -} - -func (c *Controller) MergeWith(cfg *Controller) { - c.mergeInterval(cfg) - c.mergeEndpoint(cfg) - c.mergeConditionalGathererEndpoint(cfg) - c.mergeReport(cfg) - c.mergeOCM(cfg) - c.mergeHTTP(cfg) - c.mergeProcessingStatusEndpoint(cfg) - c.mergeReportEndpointTechPreview(cfg) -} - -func (c *Controller) mergeEndpoint(cfg *Controller) { - if len(cfg.Endpoint) > 0 { - c.Endpoint = cfg.Endpoint - } -} - -func (c *Controller) mergeProcessingStatusEndpoint(cfg *Controller) { - if len(cfg.ProcessingStatusEndpoint) > 0 { - c.ProcessingStatusEndpoint = cfg.ProcessingStatusEndpoint - } -} - -func (c *Controller) mergeReportEndpointTechPreview(cfg *Controller) { - if len(cfg.ReportEndpointTechPreview) > 0 { - c.ReportEndpointTechPreview = cfg.ReportEndpointTechPreview - } -} - -func (c *Controller) mergeConditionalGathererEndpoint(cfg *Controller) { - if len(cfg.ConditionalGathererEndpoint) > 0 { - c.ConditionalGathererEndpoint = cfg.ConditionalGathererEndpoint - } -} - -func (c *Controller) mergeReport(cfg *Controller) { - if len(cfg.ReportEndpoint) > 0 { - c.ReportEndpoint = cfg.ReportEndpoint - } - if cfg.ReportPullingDelay >= 0 { - c.ReportPullingDelay = cfg.ReportPullingDelay - } - if cfg.ReportPullingTimeout > 0 { - c.ReportPullingTimeout = cfg.ReportPullingTimeout - } - if cfg.ReportMinRetryTime > 0 { - c.ReportMinRetryTime = cfg.ReportMinRetryTime - } - c.EnableGlobalObfuscation = c.EnableGlobalObfuscation || cfg.EnableGlobalObfuscation - c.DisableInsightsAlerts = c.DisableInsightsAlerts || cfg.DisableInsightsAlerts -} - -func (c *Controller) mergeOCM(cfg *Controller) { - if len(cfg.OCMConfig.SCAEndpoint) > 0 { - c.OCMConfig.SCAEndpoint = cfg.OCMConfig.SCAEndpoint - } - if cfg.OCMConfig.SCAInterval > 0 { - c.OCMConfig.SCAInterval = cfg.OCMConfig.SCAInterval - } - c.OCMConfig.SCADisabled = cfg.OCMConfig.SCADisabled - - if len(cfg.OCMConfig.ClusterTransferEndpoint) > 0 { - c.OCMConfig.ClusterTransferEndpoint = cfg.OCMConfig.ClusterTransferEndpoint +// ToConfig reads and pareses the actual serialized configuration from "InsightsConfigurationSerialized" +// and returns the "InsightsConfiguration". +func (i *InsightsConfigurationSerialized) ToConfig() *InsightsConfiguration { + ic := &InsightsConfiguration{ + DataReporting: DataReporting{ + UploadEndpoint: i.DataReporting.UploadEndpoint, + DownloadEndpoint: i.DataReporting.DownloadEndpoint, + DownloadEndpointTechPreview: i.DataReporting.DownloadEndpointTechPreview, + StoragePath: i.DataReporting.StoragePath, + ConditionalGathererEndpoint: i.DataReporting.ConditionalGathererEndpoint, + ProcessingStatusEndpoint: i.DataReporting.ProcessingStatusEndpoint, + Obfuscation: i.DataReporting.Obfuscation, + }, + SCA: SCA{ + Endpoint: i.SCA.Endpoint, + }, + ClusterTransfer: ClusterTransfer{ + Endpoint: i.ClusterTransfer.Endpoint, + }, + Proxy: Proxy{ + HTTPProxy: i.Proxy.HTTPProxy, + HTTPSProxy: i.Proxy.HTTPSProxy, + NoProxy: i.Proxy.NoProxy, + }, } - if cfg.OCMConfig.ClusterTransferInterval > 0 { - c.OCMConfig.ClusterTransferInterval = cfg.OCMConfig.ClusterTransferInterval - } -} - -func (c *Controller) mergeHTTP(cfg *Controller) { - c.HTTPConfig = cfg.HTTPConfig -} - -func (c *Controller) mergeInterval(cfg *Controller) { - if cfg.Interval > 0 { - c.Interval = cfg.Interval - } -} - -// ToController creates/updates a config Controller according to the Serialized config. -// Makes sure that the config is correct. -func ToController(s *Serialized, cfg *Controller) (*Controller, error) { // nolint: gocyclo, funlen - if cfg == nil { - cfg = &Controller{} - } - cfg.Report = s.Report - cfg.StoragePath = s.StoragePath - cfg.Endpoint = s.Endpoint - cfg.ConditionalGathererEndpoint = s.ConditionalGathererEndpoint - cfg.Impersonate = s.Impersonate - cfg.EnableGlobalObfuscation = s.EnableGlobalObfuscation - cfg.DisableInsightsAlerts = s.DisableInsightsAlerts - cfg.ProcessingStatusEndpoint = s.ProcessingStatusEndpoint - cfg.ReportEndpointTechPreview = s.ReportEndpointTechPreview - - if len(s.Interval) > 0 { - d, err := time.ParseDuration(s.Interval) - if err != nil { - return nil, fmt.Errorf("interval must be a valid duration: %v", err) - } - cfg.Interval = d - } - - if cfg.Interval <= 0 { - return nil, fmt.Errorf("interval must be a non-negative duration") - } - - if len(s.PullReport.Endpoint) > 0 { - cfg.ReportEndpoint = s.PullReport.Endpoint - } - - if len(s.PullReport.Delay) > 0 { - d, err := time.ParseDuration(s.PullReport.Delay) - if err != nil { - return nil, fmt.Errorf("delay must be a valid duration: %v", err) - } - cfg.ReportPullingDelay = d - } - - if cfg.ReportPullingDelay <= 0 { - return nil, fmt.Errorf("delay must be a non-negative duration") - } - - if len(s.PullReport.MinRetryTime) > 0 { - d, err := time.ParseDuration(s.PullReport.MinRetryTime) - if err != nil { - return nil, fmt.Errorf("min_retry must be a valid duration: %v", err) - } - cfg.ReportMinRetryTime = d - } - - if cfg.ReportMinRetryTime <= 0 { - return nil, fmt.Errorf("min_retry must be a non-negative duration") + if i.DataReporting.Interval != "" { + ic.DataReporting.Interval = parseInterval(i.DataReporting.Interval, defaultGatherFrequency) } - if len(s.PullReport.Timeout) > 0 { - d, err := time.ParseDuration(s.PullReport.Timeout) - if err != nil { - return nil, fmt.Errorf("timeout must be a valid duration: %v", err) - } - cfg.ReportPullingTimeout = d + if i.SCA.Interval != "" { + ic.SCA.Interval = parseInterval(i.SCA.Interval, defaultSCAFfrequency) } - if cfg.ReportPullingTimeout <= 0 { - return nil, fmt.Errorf("timeout must be a non-negative duration") + if i.ClusterTransfer.Interval != "" { + ic.ClusterTransfer.Interval = parseInterval(i.ClusterTransfer.Interval, defaultClusterTransferFrequency) } - if len(cfg.StoragePath) == 0 { - return nil, fmt.Errorf("storagePath must point to a directory where snapshots can be stored") + if i.Alerting.Disabled != "" { + ic.Alerting.Disabled = strings.EqualFold(i.Alerting.Disabled, "true") } - if len(s.OCM.SCAEndpoint) > 0 { - cfg.OCMConfig.SCAEndpoint = s.OCM.SCAEndpoint + if i.SCA.Disabled != "" { + ic.SCA.Disabled = strings.EqualFold(i.SCA.Disabled, "true") } - cfg.OCMConfig.SCADisabled = s.OCM.SCADisabled - if len(s.OCM.SCAInterval) > 0 { - i, err := time.ParseDuration(s.OCM.SCAInterval) - if err != nil { - return nil, fmt.Errorf("OCM SCA interval must be a valid duration: %v", err) - } - cfg.OCMConfig.SCAInterval = i - } - if len(s.OCM.ClusterTransferEndpoint) > 0 { - cfg.OCMConfig.ClusterTransferEndpoint = s.OCM.ClusterTransferEndpoint - } - if len(s.OCM.ClusterTransferInterval) > 0 { - i, err := time.ParseDuration(s.OCM.ClusterTransferInterval) - if err != nil { - return nil, fmt.Errorf("OCM Cluster transfer interval must be a valid duration: %v", err) - } - cfg.OCMConfig.ClusterTransferInterval = i - } - return cfg, nil + return ic } -// ToDisconnectedController creates/updates a config Controller according to the Serialized config. -// Makes sure that the config is correct, but only checks fields necessary for disconnected operation. -func ToDisconnectedController(s *Serialized, cfg *Controller) (*Controller, error) { - if cfg == nil { - cfg = &Controller{} - } - cfg.Report = s.Report - cfg.StoragePath = s.StoragePath - cfg.Impersonate = s.Impersonate - cfg.EnableGlobalObfuscation = s.EnableGlobalObfuscation - cfg.ConditionalGathererEndpoint = s.ConditionalGathererEndpoint - cfg.DisableInsightsAlerts = s.DisableInsightsAlerts - - if len(s.Interval) > 0 { - d, err := time.ParseDuration(s.Interval) - if err != nil { - return nil, fmt.Errorf("interval must be a valid duration: %v", err) - } - cfg.Interval = d - } - - if cfg.Interval <= 0 { - return nil, fmt.Errorf("interval must be a non-negative duration") - } - - if len(cfg.StoragePath) == 0 { - return nil, fmt.Errorf("storagePath must point to a directory where snapshots can be stored") - } - return cfg, nil -} - -// LoadConfig unmarshalls config from obj and loads it to this Controller struct -func LoadConfig(controller Controller, obj map[string]interface{}, converter Converter) (Controller, error) { //nolint: gocritic - var cfg Serialized - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj, &cfg); err != nil { - return controller, fmt.Errorf("unable to load config: %v", err) - } - - loadedController, err := converter(&cfg, &controller) +// parseInterval tries to parse the "interval" string as time duration and if there is an error +// or negative time value then the provided default time duration is used +func parseInterval(interval string, defaultValue time.Duration) time.Duration { + durationInt, err := time.ParseDuration(interval) if err != nil { - return controller, err - } - data, _ := json.Marshal(cfg) - klog.V(2).Infof("Current config: %s", string(data)) - return *loadedController, nil + klog.Errorf("Cannot parse interval time duration: %v. Using default value %s", err, defaultValue) + return defaultValue + } + if durationInt <= 0 { + durationInt = defaultValue + } + return durationInt +} + +func (d *DataReporting) String() string { + s := fmt.Sprintf(` + interval: %s, + uploadEndpoint: %s, + storagePath: %s, + downloadEndpoint: %s, + conditionalGathererEndpoint: %s, + obfuscation: %s`, + d.Interval, + d.UploadEndpoint, + d.StoragePath, + d.DownloadEndpoint, + d.ConditionalGathererEndpoint, + d.Obfuscation) + return s +} + +func (s *SCA) String() string { + str := fmt.Sprintf(` + disabled: %v, + endpoint: %s, + interval: %s`, + s.Disabled, + s.Endpoint, + s.Interval) + return str +} + +func (a *Alerting) String() string { + s := fmt.Sprintf(` + disabled: %v`, a.Disabled) + return s +} + +func (p *Proxy) String() string { + s := fmt.Sprintf(` + httpProxy: %s, + httpsProxy: %s, + noProxy: %s`, + p.HTTPProxy, + p.HTTPSProxy, + p.NoProxy) + return s +} + +func (c *ClusterTransfer) String() string { + s := fmt.Sprintf(` + endpoint: %s, + interval: %s`, + c.Endpoint, + c.Interval) + return s +} + +func (i *InsightsConfiguration) String() string { + s := fmt.Sprintf(` + dataReporting:%s + sca:%s + alerting:%s + clusterTransfer:%s + proxy:%s`, + i.DataReporting.String(), + i.SCA.String(), + i.Alerting.String(), + i.ClusterTransfer.String(), + i.Proxy.String(), + ) + return s } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index f83ae7ef2..9dcf9340e 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1,209 +1,103 @@ package config import ( - "fmt" "testing" "time" "github.com/stretchr/testify/assert" ) -func TestLoadConfig(t *testing.T) { +func TestToConfig(t *testing.T) { tests := []struct { - name string - ctrl Controller - obj map[string]interface{} - expectedOutput Controller - err error + name string + serializedConfig InsightsConfigurationSerialized + config *InsightsConfiguration }{ { - name: "controller defaults are overwritten by the serialized config", - ctrl: Controller{ - Endpoint: "default-endpoint", - Report: false, - Interval: 5 * time.Minute, - StoragePath: "default-storage-path", - ReportEndpoint: "default-report-endpoint", - ReportPullingDelay: 30 * time.Second, - ReportMinRetryTime: 60 * time.Second, - ReportPullingTimeout: 2 * time.Minute, - OCMConfig: OCMConfig{ - SCAInterval: 1 * time.Hour, - SCAEndpoint: "default-sca-endpoint", - ClusterTransferEndpoint: "default-ct-endpoint", - ClusterTransferInterval: 24 * time.Hour, + name: "basic test", + serializedConfig: InsightsConfigurationSerialized{ + DataReporting: DataReportingSerialized{ + Interval: "5m", + UploadEndpoint: "test.upload.endpoint/v1", + StoragePath: "/tmp/test/path", + Obfuscation: Obfuscation{ + Networking, + WorkloadNames, + }, }, - }, - obj: map[string]interface{}{ - "report": true, - "interval": "2h", - "endpoint": "real-endpoint", - "storagePath": "/tmp/insights-operator", - "pull_report": map[string]interface{}{ - "delay": "1m", - "min_retry": "5m", - "endpoint": "real-pull-report-endpoint", - "timeout": "4m", + SCA: SCASerialized{ + Disabled: "true", + Interval: "12h", + Endpoint: "test.sca.endpoint", }, - "ocm": map[string]interface{}{ - "scaInterval": "8h", - "scaEndpoint": "real-sca-endpoint", - "clusterTransferEndpoint": "real-ct-endpoint", - "clusterTransferInterval": "12h", + ClusterTransfer: ClusterTransferSerialized{ + Interval: "14h", }, - }, - expectedOutput: Controller{ - Report: true, - Interval: 2 * time.Hour, - Endpoint: "real-endpoint", - StoragePath: "/tmp/insights-operator", - ReportEndpoint: "real-pull-report-endpoint", - ReportPullingDelay: 1 * time.Minute, - ReportMinRetryTime: 5 * time.Minute, - ReportPullingTimeout: 4 * time.Minute, - OCMConfig: OCMConfig{ - SCAInterval: 8 * time.Hour, - SCAEndpoint: "real-sca-endpoint", - ClusterTransferEndpoint: "real-ct-endpoint", - ClusterTransferInterval: 12 * time.Hour, + Alerting: AlertingSerialized{ + Disabled: "false", }, }, - err: nil, - }, - { - name: "interval cannot be empty", - ctrl: Controller{}, - obj: map[string]interface{}{}, - expectedOutput: Controller{}, - err: fmt.Errorf("interval must be a non-negative duration"), - }, - { - name: "interval must be valid duration", - ctrl: Controller{}, - obj: map[string]interface{}{ - "interval": "notnumber", - }, - expectedOutput: Controller{ - Interval: 0, - }, - err: fmt.Errorf("interval must be a valid duration: time: invalid duration \"notnumber\""), - }, - { - name: "delay cannot be empty", - ctrl: Controller{}, - obj: map[string]interface{}{ - "interval": "2h", - }, - expectedOutput: Controller{ - Interval: 2 * time.Hour, - }, - err: fmt.Errorf("delay must be a non-negative duration"), - }, - { - name: "min_retry cannot be empty", - ctrl: Controller{}, - obj: map[string]interface{}{ - "interval": "2h", - "pull_report": map[string]interface{}{ - "delay": "1m", + config: &InsightsConfiguration{ + DataReporting: DataReporting{ + Interval: 5 * time.Minute, + UploadEndpoint: "test.upload.endpoint/v1", + StoragePath: "/tmp/test/path", + Obfuscation: Obfuscation{ + Networking, + WorkloadNames, + }, }, - }, - expectedOutput: Controller{ - Interval: 2 * time.Hour, - ReportPullingDelay: 1 * time.Minute, - }, - err: fmt.Errorf("min_retry must be a non-negative duration"), - }, - { - name: "timeout cannot be empty", - ctrl: Controller{}, - obj: map[string]interface{}{ - "interval": "2h", - "pull_report": map[string]interface{}{ - "delay": "1m", - "min_retry": "2m", + SCA: SCA{ + Disabled: true, + Interval: 12 * time.Hour, + Endpoint: "test.sca.endpoint", + }, + ClusterTransfer: ClusterTransfer{ + Interval: 14 * time.Hour, }, }, - expectedOutput: Controller{ - Interval: 2 * time.Hour, - ReportPullingDelay: 1 * time.Minute, - ReportMinRetryTime: 2 * time.Minute, - }, - err: fmt.Errorf("timeout must be a non-negative duration"), }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testConfig := tt.serializedConfig.ToConfig() + assert.Equal(t, tt.config, testConfig) + }) + } +} + +func TestParseInterval(t *testing.T) { + tests := []struct { + name string + intervalString string + defaultValue time.Duration + expectedInterval time.Duration + }{ { - name: "storagePath cannot be empty", - ctrl: Controller{}, - obj: map[string]interface{}{ - "interval": "2h", - "pull_report": map[string]interface{}{ - "delay": "1m", - "min_retry": "2m", - "timeout": "5m", - }, - }, - expectedOutput: Controller{ - Interval: 2 * time.Hour, - ReportPullingDelay: 1 * time.Minute, - ReportMinRetryTime: 2 * time.Minute, - ReportPullingTimeout: 5 * time.Minute, - }, - err: fmt.Errorf("storagePath must point to a directory where snapshots can be stored"), + name: "basic test with meaningful interval value", + intervalString: "1h", + defaultValue: 30 * time.Minute, + expectedInterval: 1 * time.Hour, }, { - name: "SCA interval must be valid duration", - ctrl: Controller{}, - obj: map[string]interface{}{ - "interval": "2h", - "pull_report": map[string]interface{}{ - "delay": "1m", - "min_retry": "2m", - "timeout": "5m", - }, - "storagePath": "test/path", - "ocm": map[string]interface{}{ - "scaInterval": "not-duration", - }, - }, - expectedOutput: Controller{ - Interval: 2 * time.Hour, - ReportPullingDelay: 1 * time.Minute, - ReportMinRetryTime: 2 * time.Minute, - ReportPullingTimeout: 5 * time.Minute, - StoragePath: "test/path", - }, - err: fmt.Errorf("OCM SCA interval must be a valid duration: time: invalid duration \"not-duration\""), + name: "interval cannot be parsed", + intervalString: "not a duration", + defaultValue: 30 * time.Minute, + expectedInterval: 30 * time.Minute, }, { - name: "SCA interval must be valid duration", - ctrl: Controller{}, - obj: map[string]interface{}{ - "interval": "2h", - "pull_report": map[string]interface{}{ - "delay": "1m", - "min_retry": "2m", - "timeout": "5m", - }, - "storagePath": "test/path", - "ocm": map[string]interface{}{ - "clusterTransferInterval": "not-duration", - }, - }, - expectedOutput: Controller{ - Interval: 2 * time.Hour, - ReportPullingDelay: 1 * time.Minute, - ReportMinRetryTime: 2 * time.Minute, - ReportPullingTimeout: 5 * time.Minute, - StoragePath: "test/path", - }, - err: fmt.Errorf("OCM Cluster transfer interval must be a valid duration: time: invalid duration \"not-duration\""), + name: "interval is negative duration", + intervalString: "-10m", + defaultValue: 30 * time.Minute, + expectedInterval: 30 * time.Minute, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - output, err := LoadConfig(tt.ctrl, tt.obj, ToController) - assert.Equal(t, tt.err, err) - assert.Equal(t, tt.expectedOutput, output) + interval := parseInterval(tt.intervalString, tt.defaultValue) + assert.Equal(t, tt.expectedInterval, interval) }) } } diff --git a/pkg/config/configobserver/config_aggregator.go b/pkg/config/configobserver/config_aggregator.go index 5c69fa00d..f32b0db09 100644 --- a/pkg/config/configobserver/config_aggregator.go +++ b/pkg/config/configobserver/config_aggregator.go @@ -101,7 +101,19 @@ func (c *ConfigAggregator) mergeStatically() { c.merge(conf, cmConf) } +// merge merges the default configuration options with the defined ones func (c *ConfigAggregator) merge(defaultCfg, newCfg *config.InsightsConfiguration) { + c.mergeDataReporting(defaultCfg, newCfg) + c.mergeSCAConfig(defaultCfg, newCfg) + c.mergeAlerting(defaultCfg, newCfg) + c.mergeClusterTransfer(defaultCfg, newCfg) + c.mergeProxyConfig(defaultCfg, newCfg) + c.config = defaultCfg +} + +// mergeDataReporting checks configured data reporting options and if they are not empty then +// override default data reporting configuration +func (c *ConfigAggregator) mergeDataReporting(defaultCfg, newCfg *config.InsightsConfiguration) { // read config map values and merge if newCfg.DataReporting.Interval != 0 { defaultCfg.DataReporting.Interval = newCfg.DataReporting.Interval @@ -134,7 +146,56 @@ func (c *ConfigAggregator) merge(defaultCfg, newCfg *config.InsightsConfiguratio if len(newCfg.DataReporting.Obfuscation) > 0 { defaultCfg.DataReporting.Obfuscation = append(defaultCfg.DataReporting.Obfuscation, newCfg.DataReporting.Obfuscation...) } - c.config = defaultCfg +} + +func (c *ConfigAggregator) mergeAlerting(defaultCfg, newCfg *config.InsightsConfiguration) { + if newCfg.Alerting.Disabled != defaultCfg.Alerting.Disabled { + defaultCfg.Alerting.Disabled = newCfg.Alerting.Disabled + } +} + +// mergeSCAConfig checks configured SCA options and if they are not empty then +// override default SCA configuration +func (c *ConfigAggregator) mergeSCAConfig(defaultCfg, newCfg *config.InsightsConfiguration) { + if newCfg.SCA.Interval != 0 { + defaultCfg.SCA.Interval = newCfg.SCA.Interval + } + + if newCfg.SCA.Endpoint != "" { + defaultCfg.SCA.Endpoint = newCfg.SCA.Endpoint + } + + if newCfg.SCA.Disabled != defaultCfg.SCA.Disabled { + defaultCfg.SCA.Disabled = newCfg.SCA.Disabled + } +} + +// mergeProxyConfig checks configured proxy options and if they are not empty then +// override default connection configuration +func (c *ConfigAggregator) mergeProxyConfig(defaultCfg, newCfg *config.InsightsConfiguration) { + if newCfg.Proxy.HTTPProxy != "" { + defaultCfg.Proxy.HTTPProxy = newCfg.Proxy.HTTPProxy + } + + if newCfg.Proxy.HTTPSProxy != "" { + defaultCfg.Proxy.HTTPSProxy = newCfg.Proxy.HTTPSProxy + } + + if newCfg.Proxy.NoProxy != "" { + defaultCfg.Proxy.NoProxy = newCfg.Proxy.NoProxy + } +} + +// mergeClusterTransfer checks configured cluster transfer options and if they are not empty then +// override default cluster transfer configuration +func (c *ConfigAggregator) mergeClusterTransfer(defaultCfg, newCfg *config.InsightsConfiguration) { + if newCfg.ClusterTransfer.Interval != 0 { + defaultCfg.ClusterTransfer.Interval = newCfg.ClusterTransfer.Interval + } + + if newCfg.ClusterTransfer.Endpoint != "" { + defaultCfg.ClusterTransfer.Endpoint = newCfg.ClusterTransfer.Endpoint + } } func (c *ConfigAggregator) Config() *config.InsightsConfiguration { @@ -211,5 +272,22 @@ func (c *ConfigAggregator) legacyConfigToInsightsConfiguration() *config.Insight ReportPullingDelay: legacyConfig.ReportPullingDelay, Obfuscation: obfuscation, }, + Alerting: config.Alerting{ + Disabled: legacyConfig.DisableInsightsAlerts, + }, + SCA: config.SCA{ + Disabled: legacyConfig.OCMConfig.SCADisabled, + Interval: legacyConfig.OCMConfig.SCAInterval, + Endpoint: legacyConfig.OCMConfig.SCAEndpoint, + }, + ClusterTransfer: config.ClusterTransfer{ + Interval: legacyConfig.OCMConfig.ClusterTransferInterval, + Endpoint: legacyConfig.OCMConfig.ClusterTransferEndpoint, + }, + Proxy: config.Proxy{ + HTTPProxy: legacyConfig.HTTPConfig.HTTPProxy, + HTTPSProxy: legacyConfig.HTTPConfig.HTTPSProxy, + NoProxy: legacyConfig.HTTPConfig.NoProxy, + }, } } diff --git a/pkg/config/configobserver/config_aggregator_test.go b/pkg/config/configobserver/config_aggregator_test.go index c44ec8f02..04e99d135 100644 --- a/pkg/config/configobserver/config_aggregator_test.go +++ b/pkg/config/configobserver/config_aggregator_test.go @@ -30,6 +30,12 @@ func TestMergeStatically(t *testing.T) { Interval: 2 * time.Hour, ConditionalGathererEndpoint: "http://conditionalendpoint.here", EnableGlobalObfuscation: true, + DisableInsightsAlerts: true, + OCMConfig: config.OCMConfig{ + SCAInterval: 5 * time.Hour, + SCAEndpoint: "test.sca.endpoint", + SCADisabled: true, + }, }, expectedConfig: &config.InsightsConfiguration{ DataReporting: config.DataReporting{ @@ -41,6 +47,14 @@ func TestMergeStatically(t *testing.T) { ConditionalGathererEndpoint: "http://conditionalendpoint.here", Obfuscation: config.Obfuscation{config.Networking}, }, + SCA: config.SCA{ + Interval: 5 * time.Hour, + Endpoint: "test.sca.endpoint", + Disabled: true, + }, + Alerting: config.Alerting{ + Disabled: true, + }, }, }, { @@ -61,7 +75,15 @@ dataReporting: processingStatusEndpoint: https://overriden.status/endpoint downloadEndpointTechPreview: https://overriden.downloadtechpreview/endpoint obfuscation: - - workload_names`, + - workload_names +alerting: + disabled: true +sca: + disabled: true + endpoint: updated.sca.endpoint +clusterTransfer: + interval: 12h + endpoint: cluster.transfer.endpoint.overriden`, }, }, legacyConfig: config.Controller{ @@ -74,6 +96,14 @@ dataReporting: ProcessingStatusEndpoint: "http://statusendpoint.here", ReportEndpointTechPreview: "http://downloadtpendpoint.here", EnableGlobalObfuscation: true, + DisableInsightsAlerts: false, + OCMConfig: config.OCMConfig{ + SCAInterval: 5 * time.Hour, + SCAEndpoint: "test.sca.endpoint", + SCADisabled: false, + ClusterTransferEndpoint: "cluster.transfer.endpoint", + ClusterTransferInterval: 10 * time.Hour, + }, }, expectedConfig: &config.InsightsConfiguration{ DataReporting: config.DataReporting{ @@ -87,10 +117,22 @@ dataReporting: DownloadEndpointTechPreview: "https://overriden.downloadtechpreview/endpoint", Obfuscation: config.Obfuscation{config.Networking, config.WorkloadNames}, }, + Alerting: config.Alerting{ + Disabled: true, + }, + SCA: config.SCA{ + Disabled: true, + Interval: 5 * time.Hour, + Endpoint: "updated.sca.endpoint", + }, + ClusterTransfer: config.ClusterTransfer{ + Interval: 12 * time.Hour, + Endpoint: "cluster.transfer.endpoint.overriden", + }, }, }, { - name: "Config map cannot override \"Report\" bool attribute", + name: "Config map cannot override \"report\" bool attribute", configCM: &v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: insightsConfigMapName, @@ -114,6 +156,62 @@ dataReporting: }, }, }, + { + name: "Empty config also overrides the legacy config with zero values", + configCM: &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: insightsConfigMapName, + Namespace: "openshift-insights", + }, + Data: map[string]string{ + "config.yaml": ``, + }, + }, + legacyConfig: config.Controller{ + Report: true, + StoragePath: "/foo/bar/", + Endpoint: "http://testing.here", + ReportEndpoint: "http://reportendpoint.here", + Interval: 2 * time.Hour, + ConditionalGathererEndpoint: "http://conditionalendpoint.here", + ProcessingStatusEndpoint: "http://statusendpoint.here", + ReportEndpointTechPreview: "http://downloadtpendpoint.here", + EnableGlobalObfuscation: true, + DisableInsightsAlerts: true, + OCMConfig: config.OCMConfig{ + SCAInterval: 5 * time.Hour, + SCAEndpoint: "test.sca.endpoint", + SCADisabled: true, + ClusterTransferEndpoint: "cluster.transfer.endpoint", + ClusterTransferInterval: 10 * time.Hour, + }, + }, + expectedConfig: &config.InsightsConfiguration{ + DataReporting: config.DataReporting{ + Enabled: true, + UploadEndpoint: "http://testing.here", + StoragePath: "/foo/bar/", + DownloadEndpoint: "http://reportendpoint.here", + Interval: 2 * time.Hour, + ConditionalGathererEndpoint: "http://conditionalendpoint.here", + ProcessingStatusEndpoint: "http://statusendpoint.here", + DownloadEndpointTechPreview: "http://downloadtpendpoint.here", + Obfuscation: config.Obfuscation{config.Networking}, + }, + Alerting: config.Alerting{ + Disabled: false, + }, + SCA: config.SCA{ + Disabled: false, + Interval: 5 * time.Hour, + Endpoint: "test.sca.endpoint", + }, + ClusterTransfer: config.ClusterTransfer{ + Interval: 10 * time.Hour, + Endpoint: "cluster.transfer.endpoint", + }, + }, + }, } for _, tt := range tests { @@ -136,13 +234,13 @@ dataReporting: func TestMergeUsingInformer(t *testing.T) { tests := []struct { name string - configFromInf config.InsightsConfiguration + configMap *v1.ConfigMap legacyConfig config.Controller expectedConfig *config.InsightsConfiguration }{ { - name: "No config map exists - legacy config is used", - configFromInf: config.InsightsConfiguration{}, + name: "No config map exists - legacy config is used", + configMap: nil, legacyConfig: config.Controller{ Report: true, StoragePath: "/foo/bar/", @@ -151,6 +249,17 @@ func TestMergeUsingInformer(t *testing.T) { Interval: 2 * time.Hour, ConditionalGathererEndpoint: "http://conditionalendpoint.here", EnableGlobalObfuscation: true, + DisableInsightsAlerts: true, + HTTPConfig: config.HTTPConfig{ + HTTPProxy: "http://test.proxy", + HTTPSProxy: "https://test.proxy", + NoProxy: "https://no.proxy", + }, + OCMConfig: config.OCMConfig{ + SCAInterval: 5 * time.Hour, + SCAEndpoint: "test.sca.endpoint", + SCADisabled: true, + }, }, expectedConfig: &config.InsightsConfiguration{ DataReporting: config.DataReporting{ @@ -162,18 +271,53 @@ func TestMergeUsingInformer(t *testing.T) { ConditionalGathererEndpoint: "http://conditionalendpoint.here", Obfuscation: config.Obfuscation{config.Networking}, }, + Alerting: config.Alerting{ + Disabled: true, + }, + Proxy: config.Proxy{ + HTTPProxy: "http://test.proxy", + HTTPSProxy: "https://test.proxy", + NoProxy: "https://no.proxy", + }, + SCA: config.SCA{ + Disabled: true, + Endpoint: "test.sca.endpoint", + Interval: 5 * time.Hour, + }, }, }, { name: "Config map exists and overrides legacy config", - configFromInf: config.InsightsConfiguration{ - DataReporting: config.DataReporting{ - Interval: 1 * time.Hour, - UploadEndpoint: "https://overriden.upload/endpoint", - StoragePath: "/var/lib/test", - DownloadEndpoint: "https://overriden.download/endpoint", - ConditionalGathererEndpoint: "https://overriden.conditional/endpoint", - Obfuscation: config.Obfuscation{config.WorkloadNames}, + configMap: &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: insightsConfigMapName, + Namespace: "openshift-insights", + }, + Data: map[string]string{ + "config.yaml": ` +dataReporting: + interval: 1h + uploadEndpoint: https://overriden.upload/endpoint + storagePath: /var/lib/test + downloadEndpoint: https://overriden.download/endpoint + conditionalGathererEndpoint: https://overriden.conditional/endpoint + processingStatusEndpoint: https://overriden.status/endpoint + downloadEndpointTechPreview: https://overriden.downloadtechpreview/endpoint + obfuscation: + - workload_names +alerting: + disabled: true +sca: + disabled: true + endpoint: updated.sca.endpoint + interval: 8h +clusterTransfer: + interval: 12h + endpoint: cluster.transfer.endpoint.overriden +proxy: + httpProxy: http://test.proxy.updated + httpsProxy: https://test.proxy.updated + noProxy: https://no.proxy.updated`, }, }, legacyConfig: config.Controller{ @@ -184,6 +328,19 @@ func TestMergeUsingInformer(t *testing.T) { Interval: 2 * time.Hour, ConditionalGathererEndpoint: "http://conditionalendpoint.here", EnableGlobalObfuscation: true, + DisableInsightsAlerts: false, + HTTPConfig: config.HTTPConfig{ + HTTPProxy: "http://test.proxy", + HTTPSProxy: "https://test.proxy", + NoProxy: "https://no.proxy", + }, + OCMConfig: config.OCMConfig{ + SCAInterval: 4 * time.Hour, + SCAEndpoint: "endpoint", + SCADisabled: true, + ClusterTransferEndpoint: "cluster.transfer.endpoint", + ClusterTransferInterval: 10 * time.Hour, + }, }, expectedConfig: &config.InsightsConfiguration{ DataReporting: config.DataReporting{ @@ -191,18 +348,43 @@ func TestMergeUsingInformer(t *testing.T) { UploadEndpoint: "https://overriden.upload/endpoint", StoragePath: "/var/lib/test", DownloadEndpoint: "https://overriden.download/endpoint", + DownloadEndpointTechPreview: "https://overriden.downloadtechpreview/endpoint", + ProcessingStatusEndpoint: "https://overriden.status/endpoint", Interval: 1 * time.Hour, ConditionalGathererEndpoint: "https://overriden.conditional/endpoint", Obfuscation: config.Obfuscation{config.Networking, config.WorkloadNames}, }, + Alerting: config.Alerting{ + Disabled: true, + }, + Proxy: config.Proxy{ + HTTPProxy: "http://test.proxy.updated", + HTTPSProxy: "https://test.proxy.updated", + NoProxy: "https://no.proxy.updated", + }, + SCA: config.SCA{ + Disabled: true, + Endpoint: "updated.sca.endpoint", + Interval: 8 * time.Hour, + }, + ClusterTransfer: config.ClusterTransfer{ + Interval: 12 * time.Hour, + Endpoint: "cluster.transfer.endpoint.overriden", + }, }, }, { name: "Config map cannot override \"Report\" bool attribute", - configFromInf: config.InsightsConfiguration{ - DataReporting: config.DataReporting{ - Enabled: true, - UploadEndpoint: "https://overriden.upload/endpoint", + configMap: &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: insightsConfigMapName, + Namespace: "openshift-insights", + }, + Data: map[string]string{ + "config.yaml": ` +dataReporting: + enabled: true + uploadEndpoint: https://overriden.upload/endpoint`, }, }, legacyConfig: config.Controller{ @@ -216,12 +398,55 @@ func TestMergeUsingInformer(t *testing.T) { }, }, }, + { + name: "Empty config also overrides the legacy config with zero values", + configMap: &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: insightsConfigMapName, + Namespace: "openshift-insights", + }, + Data: map[string]string{ + "config.yaml": ``, + }, + }, + legacyConfig: config.Controller{ + Report: true, + Endpoint: "http://testing.here", + ReportEndpoint: "http://reportendpoint.here", + Interval: 2 * time.Hour, + EnableGlobalObfuscation: true, + DisableInsightsAlerts: true, + OCMConfig: config.OCMConfig{ + SCAInterval: 5 * time.Hour, + SCAEndpoint: "test.sca.endpoint", + SCADisabled: true, + }, + }, + expectedConfig: &config.InsightsConfiguration{ + DataReporting: config.DataReporting{ + Enabled: true, + UploadEndpoint: "http://testing.here", + DownloadEndpoint: "http://reportendpoint.here", + Interval: 2 * time.Hour, + Obfuscation: config.Obfuscation{config.Networking}, + }, + Alerting: config.Alerting{ + Disabled: false, // this was not provide in the empty config map and zero value (false) is used + }, + SCA: config.SCA{ + Disabled: false, // this was not provide in the empty config map and zero value (false) is used + Endpoint: "test.sca.endpoint", + Interval: 5 * time.Hour, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { mockSecretConf := config.NewMockSecretConfigurator(&tt.legacyConfig) - mockConfigMapInf := NewMockConfigMapInformer(&tt.configFromInf) + mockConfigMapInf, err := NewMockConfigMapInformer(tt.configMap) + assert.NoError(t, err) informerAggregator := NewConfigAggregator(mockSecretConf, mockConfigMapInf) testConfig := informerAggregator.Config() @@ -235,10 +460,20 @@ type MockConfigMapInformer struct { config *config.InsightsConfiguration } -func NewMockConfigMapInformer(cfg *config.InsightsConfiguration) *MockConfigMapInformer { +func NewMockConfigMapInformer(cm *v1.ConfigMap) (*MockConfigMapInformer, error) { + if cm == nil { + return &MockConfigMapInformer{ + config: nil, + }, nil + } + + cfg, err := readConfigAndDecode(cm) + if err != nil { + return nil, err + } return &MockConfigMapInformer{ config: cfg, - } + }, nil } func (m *MockConfigMapInformer) Config() *config.InsightsConfiguration { diff --git a/pkg/config/configobserver/configmapobserver.go b/pkg/config/configobserver/configmapobserver.go index 14ceeccfc..a82f29c32 100644 --- a/pkg/config/configobserver/configmapobserver.go +++ b/pkg/config/configobserver/configmapobserver.go @@ -3,7 +3,6 @@ package configobserver import ( "context" "sync" - "time" "github.com/openshift/insights-operator/pkg/config" "github.com/openshift/library-go/pkg/controller/factory" @@ -56,7 +55,6 @@ func NewConfigMapObserver(kubeConfig *rest.Config, } factoryCtrl := factory.New().WithInformers(cmInformer). WithSync(ctrl.sync). - ResyncEvery(10*time.Minute). ToController("ConfigController", eventRecorder) ctrl.Controller = factoryCtrl diff --git a/pkg/config/configobserver/secretconfigobserver.go b/pkg/config/configobserver/secretconfigobserver.go index 65d2abe8a..e456d5c98 100644 --- a/pkg/config/configobserver/secretconfigobserver.go +++ b/pkg/config/configobserver/secretconfigobserver.go @@ -159,7 +159,7 @@ func (c *Controller) updateToken(ctx context.Context) error { // Updates the stored configs from the secrets in the cluster. (if present) func (c *Controller) updateConfig(ctx context.Context) error { - klog.V(2).Infof("Refreshing configuration from cluster secret") + klog.V(2).Infof("Refreshing configuration from cluster support secret") secret, err := c.fetchSecret(ctx, "support") if err != nil { return err @@ -200,7 +200,7 @@ func (c *Controller) setSecretConfig(operatorConfig *config.Controller) { func (c *Controller) setConfig(operatorConfig *config.Controller) { if c.config != nil { if !reflect.DeepEqual(c.config, operatorConfig) { - klog.V(2).Infof("Configuration updated: %s", operatorConfig.ToString()) + klog.V(2).Infof("Legacy configuration updated: %s", operatorConfig.ToString()) for _, ch := range c.listeners { if ch == nil { continue @@ -212,7 +212,7 @@ func (c *Controller) setConfig(operatorConfig *config.Controller) { } } } else { - klog.V(2).Infof("Configuration set: %s", operatorConfig.ToString()) + klog.V(2).Infof("Legacy configuration set: %s", operatorConfig.ToString()) } c.config = operatorConfig } diff --git a/pkg/config/legacy_config.go b/pkg/config/legacy_config.go new file mode 100644 index 000000000..f54e1577d --- /dev/null +++ b/pkg/config/legacy_config.go @@ -0,0 +1,329 @@ +package config + +import ( + "encoding/json" + "fmt" + "time" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/klog/v2" +) + +// Serialized defines the standard config for this operator. +type Serialized struct { + Report bool `json:"report"` + StoragePath string `json:"storagePath"` + Interval string `json:"interval"` + Endpoint string `json:"endpoint"` + ConditionalGathererEndpoint string `json:"conditionalGathererEndpoint"` + PullReport struct { + Endpoint string `json:"endpoint"` + Delay string `json:"delay"` + Timeout string `json:"timeout"` + MinRetryTime string `json:"min_retry"` + } `json:"pull_report"` + Impersonate string `json:"impersonate"` + EnableGlobalObfuscation bool `json:"enableGlobalObfuscation"` + OCM struct { + SCAEndpoint string `json:"scaEndpoint"` + SCAInterval string `json:"scaInterval"` + SCADisabled bool `json:"scaDisabled"` + ClusterTransferEndpoint string `json:"clusterTransferEndpoint"` + ClusterTransferInterval string `json:"clusterTransferInterval"` + } `json:"ocm"` + DisableInsightsAlerts bool `json:"disableInsightsAlerts"` + ProcessingStatusEndpoint string `json:"processingStatusEndpoint"` + ReportEndpointTechPreview string `json:"reportEndpointTechPreview"` +} + +// Controller defines the standard config for this operator. +type Controller struct { + Report bool + StoragePath string + Interval time.Duration + Endpoint string + ConditionalGathererEndpoint string + ReportEndpoint string + ReportEndpointTechPreview string + ReportPullingDelay time.Duration + ReportMinRetryTime time.Duration + ReportPullingTimeout time.Duration + Impersonate string + // EnableGlobalObfuscation enables obfuscation of domain names and IP addresses + // To see the detailed info about how anonymization works, go to the docs of package anonymization. + EnableGlobalObfuscation bool + + Token string + + HTTPConfig HTTPConfig + OCMConfig OCMConfig + + // DisableInsightsAlerts disabled exposing of Insights recommendations as Prometheus info alerts + DisableInsightsAlerts bool + ProcessingStatusEndpoint string +} + +// HTTPConfig configures http proxy and exception settings if they come from config +type HTTPConfig struct { + HTTPProxy string + HTTPSProxy string + NoProxy string +} + +// OCMConfig configures the interval and endpoint for retrieving the data from OCM API +type OCMConfig struct { + SCAInterval time.Duration + SCAEndpoint string + SCADisabled bool + ClusterTransferEndpoint string + ClusterTransferInterval time.Duration +} + +type Converter func(s *Serialized, cfg *Controller) (*Controller, error) + +// ToString returns the important fields of the config in a string form +func (c *Controller) ToString() string { + return fmt.Sprintf("enabled=%t "+ + "endpoint=%s "+ + "conditional_gatherer_endpoint=%s "+ + "interval=%s "+ + "token=%t "+ + "reportEndpoint=%s "+ + "initialPollingDelay=%s "+ + "minRetryTime=%s "+ + "pollingTimeout=%s "+ + "processingStatusEndpoint=%s", + c.Report, + c.Endpoint, + c.ConditionalGathererEndpoint, + c.Interval, + len(c.Token) > 0, + c.ReportEndpoint, + c.ReportPullingDelay, + c.ReportMinRetryTime, + c.ReportPullingTimeout, + c.ProcessingStatusEndpoint) +} + +func (c *Controller) MergeWith(cfg *Controller) { + c.mergeInterval(cfg) + c.mergeEndpoint(cfg) + c.mergeConditionalGathererEndpoint(cfg) + c.mergeReport(cfg) + c.mergeOCM(cfg) + c.mergeHTTP(cfg) + c.mergeProcessingStatusEndpoint(cfg) + c.mergeReportEndpointTechPreview(cfg) +} + +func (c *Controller) mergeEndpoint(cfg *Controller) { + if len(cfg.Endpoint) > 0 { + c.Endpoint = cfg.Endpoint + } +} + +func (c *Controller) mergeProcessingStatusEndpoint(cfg *Controller) { + if len(cfg.ProcessingStatusEndpoint) > 0 { + c.ProcessingStatusEndpoint = cfg.ProcessingStatusEndpoint + } +} + +func (c *Controller) mergeReportEndpointTechPreview(cfg *Controller) { + if len(cfg.ReportEndpointTechPreview) > 0 { + c.ReportEndpointTechPreview = cfg.ReportEndpointTechPreview + } +} + +func (c *Controller) mergeConditionalGathererEndpoint(cfg *Controller) { + if len(cfg.ConditionalGathererEndpoint) > 0 { + c.ConditionalGathererEndpoint = cfg.ConditionalGathererEndpoint + } +} + +func (c *Controller) mergeReport(cfg *Controller) { + if len(cfg.ReportEndpoint) > 0 { + c.ReportEndpoint = cfg.ReportEndpoint + } + if cfg.ReportPullingDelay >= 0 { + c.ReportPullingDelay = cfg.ReportPullingDelay + } + if cfg.ReportPullingTimeout > 0 { + c.ReportPullingTimeout = cfg.ReportPullingTimeout + } + if cfg.ReportMinRetryTime > 0 { + c.ReportMinRetryTime = cfg.ReportMinRetryTime + } + c.EnableGlobalObfuscation = c.EnableGlobalObfuscation || cfg.EnableGlobalObfuscation + c.DisableInsightsAlerts = c.DisableInsightsAlerts || cfg.DisableInsightsAlerts +} + +func (c *Controller) mergeOCM(cfg *Controller) { + if len(cfg.OCMConfig.SCAEndpoint) > 0 { + c.OCMConfig.SCAEndpoint = cfg.OCMConfig.SCAEndpoint + } + if cfg.OCMConfig.SCAInterval > 0 { + c.OCMConfig.SCAInterval = cfg.OCMConfig.SCAInterval + } + c.OCMConfig.SCADisabled = cfg.OCMConfig.SCADisabled + + if len(cfg.OCMConfig.ClusterTransferEndpoint) > 0 { + c.OCMConfig.ClusterTransferEndpoint = cfg.OCMConfig.ClusterTransferEndpoint + } + if cfg.OCMConfig.ClusterTransferInterval > 0 { + c.OCMConfig.ClusterTransferInterval = cfg.OCMConfig.ClusterTransferInterval + } +} + +func (c *Controller) mergeHTTP(cfg *Controller) { + c.HTTPConfig = cfg.HTTPConfig +} + +func (c *Controller) mergeInterval(cfg *Controller) { + if cfg.Interval > 0 { + c.Interval = cfg.Interval + } +} + +// ToController creates/updates a config Controller according to the Serialized config. +// Makes sure that the config is correct. +func ToController(s *Serialized, cfg *Controller) (*Controller, error) { // nolint: gocyclo, funlen + if cfg == nil { + cfg = &Controller{} + } + cfg.Report = s.Report + cfg.StoragePath = s.StoragePath + cfg.Endpoint = s.Endpoint + cfg.ConditionalGathererEndpoint = s.ConditionalGathererEndpoint + cfg.Impersonate = s.Impersonate + cfg.EnableGlobalObfuscation = s.EnableGlobalObfuscation + cfg.DisableInsightsAlerts = s.DisableInsightsAlerts + cfg.ProcessingStatusEndpoint = s.ProcessingStatusEndpoint + cfg.ReportEndpointTechPreview = s.ReportEndpointTechPreview + + if len(s.Interval) > 0 { + d, err := time.ParseDuration(s.Interval) + if err != nil { + return nil, fmt.Errorf("interval must be a valid duration: %v", err) + } + cfg.Interval = d + } + + if cfg.Interval <= 0 { + return nil, fmt.Errorf("interval must be a non-negative duration") + } + + if len(s.PullReport.Endpoint) > 0 { + cfg.ReportEndpoint = s.PullReport.Endpoint + } + + if len(s.PullReport.Delay) > 0 { + d, err := time.ParseDuration(s.PullReport.Delay) + if err != nil { + return nil, fmt.Errorf("delay must be a valid duration: %v", err) + } + cfg.ReportPullingDelay = d + } + + if cfg.ReportPullingDelay <= 0 { + return nil, fmt.Errorf("delay must be a non-negative duration") + } + + if len(s.PullReport.MinRetryTime) > 0 { + d, err := time.ParseDuration(s.PullReport.MinRetryTime) + if err != nil { + return nil, fmt.Errorf("min_retry must be a valid duration: %v", err) + } + cfg.ReportMinRetryTime = d + } + + if cfg.ReportMinRetryTime <= 0 { + return nil, fmt.Errorf("min_retry must be a non-negative duration") + } + + if len(s.PullReport.Timeout) > 0 { + d, err := time.ParseDuration(s.PullReport.Timeout) + if err != nil { + return nil, fmt.Errorf("timeout must be a valid duration: %v", err) + } + cfg.ReportPullingTimeout = d + } + + if cfg.ReportPullingTimeout <= 0 { + return nil, fmt.Errorf("timeout must be a non-negative duration") + } + + if len(cfg.StoragePath) == 0 { + return nil, fmt.Errorf("storagePath must point to a directory where snapshots can be stored") + } + + if len(s.OCM.SCAEndpoint) > 0 { + cfg.OCMConfig.SCAEndpoint = s.OCM.SCAEndpoint + } + cfg.OCMConfig.SCADisabled = s.OCM.SCADisabled + + if len(s.OCM.SCAInterval) > 0 { + i, err := time.ParseDuration(s.OCM.SCAInterval) + if err != nil { + return nil, fmt.Errorf("OCM SCA interval must be a valid duration: %v", err) + } + cfg.OCMConfig.SCAInterval = i + } + if len(s.OCM.ClusterTransferEndpoint) > 0 { + cfg.OCMConfig.ClusterTransferEndpoint = s.OCM.ClusterTransferEndpoint + } + if len(s.OCM.ClusterTransferInterval) > 0 { + i, err := time.ParseDuration(s.OCM.ClusterTransferInterval) + if err != nil { + return nil, fmt.Errorf("OCM Cluster transfer interval must be a valid duration: %v", err) + } + cfg.OCMConfig.ClusterTransferInterval = i + } + return cfg, nil +} + +// ToDisconnectedController creates/updates a config Controller according to the Serialized config. +// Makes sure that the config is correct, but only checks fields necessary for disconnected operation. +func ToDisconnectedController(s *Serialized, cfg *Controller) (*Controller, error) { + if cfg == nil { + cfg = &Controller{} + } + cfg.Report = s.Report + cfg.StoragePath = s.StoragePath + cfg.Impersonate = s.Impersonate + cfg.EnableGlobalObfuscation = s.EnableGlobalObfuscation + cfg.ConditionalGathererEndpoint = s.ConditionalGathererEndpoint + cfg.DisableInsightsAlerts = s.DisableInsightsAlerts + + if len(s.Interval) > 0 { + d, err := time.ParseDuration(s.Interval) + if err != nil { + return nil, fmt.Errorf("interval must be a valid duration: %v", err) + } + cfg.Interval = d + } + + if cfg.Interval <= 0 { + return nil, fmt.Errorf("interval must be a non-negative duration") + } + + if len(cfg.StoragePath) == 0 { + return nil, fmt.Errorf("storagePath must point to a directory where snapshots can be stored") + } + return cfg, nil +} + +// LoadConfig unmarshalls config from obj and loads it to this Controller struct +func LoadConfig(controller Controller, obj map[string]interface{}, converter Converter) (Controller, error) { //nolint: gocritic + var cfg Serialized + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj, &cfg); err != nil { + return controller, fmt.Errorf("unable to load config: %v", err) + } + + loadedController, err := converter(&cfg, &controller) + if err != nil { + return controller, err + } + data, _ := json.Marshal(cfg) + klog.V(2).Infof("Current config: %s", string(data)) + return *loadedController, nil +} diff --git a/pkg/config/legacy_config_test.go b/pkg/config/legacy_config_test.go new file mode 100644 index 000000000..f83ae7ef2 --- /dev/null +++ b/pkg/config/legacy_config_test.go @@ -0,0 +1,209 @@ +package config + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestLoadConfig(t *testing.T) { + tests := []struct { + name string + ctrl Controller + obj map[string]interface{} + expectedOutput Controller + err error + }{ + { + name: "controller defaults are overwritten by the serialized config", + ctrl: Controller{ + Endpoint: "default-endpoint", + Report: false, + Interval: 5 * time.Minute, + StoragePath: "default-storage-path", + ReportEndpoint: "default-report-endpoint", + ReportPullingDelay: 30 * time.Second, + ReportMinRetryTime: 60 * time.Second, + ReportPullingTimeout: 2 * time.Minute, + OCMConfig: OCMConfig{ + SCAInterval: 1 * time.Hour, + SCAEndpoint: "default-sca-endpoint", + ClusterTransferEndpoint: "default-ct-endpoint", + ClusterTransferInterval: 24 * time.Hour, + }, + }, + obj: map[string]interface{}{ + "report": true, + "interval": "2h", + "endpoint": "real-endpoint", + "storagePath": "/tmp/insights-operator", + "pull_report": map[string]interface{}{ + "delay": "1m", + "min_retry": "5m", + "endpoint": "real-pull-report-endpoint", + "timeout": "4m", + }, + "ocm": map[string]interface{}{ + "scaInterval": "8h", + "scaEndpoint": "real-sca-endpoint", + "clusterTransferEndpoint": "real-ct-endpoint", + "clusterTransferInterval": "12h", + }, + }, + expectedOutput: Controller{ + Report: true, + Interval: 2 * time.Hour, + Endpoint: "real-endpoint", + StoragePath: "/tmp/insights-operator", + ReportEndpoint: "real-pull-report-endpoint", + ReportPullingDelay: 1 * time.Minute, + ReportMinRetryTime: 5 * time.Minute, + ReportPullingTimeout: 4 * time.Minute, + OCMConfig: OCMConfig{ + SCAInterval: 8 * time.Hour, + SCAEndpoint: "real-sca-endpoint", + ClusterTransferEndpoint: "real-ct-endpoint", + ClusterTransferInterval: 12 * time.Hour, + }, + }, + err: nil, + }, + { + name: "interval cannot be empty", + ctrl: Controller{}, + obj: map[string]interface{}{}, + expectedOutput: Controller{}, + err: fmt.Errorf("interval must be a non-negative duration"), + }, + { + name: "interval must be valid duration", + ctrl: Controller{}, + obj: map[string]interface{}{ + "interval": "notnumber", + }, + expectedOutput: Controller{ + Interval: 0, + }, + err: fmt.Errorf("interval must be a valid duration: time: invalid duration \"notnumber\""), + }, + { + name: "delay cannot be empty", + ctrl: Controller{}, + obj: map[string]interface{}{ + "interval": "2h", + }, + expectedOutput: Controller{ + Interval: 2 * time.Hour, + }, + err: fmt.Errorf("delay must be a non-negative duration"), + }, + { + name: "min_retry cannot be empty", + ctrl: Controller{}, + obj: map[string]interface{}{ + "interval": "2h", + "pull_report": map[string]interface{}{ + "delay": "1m", + }, + }, + expectedOutput: Controller{ + Interval: 2 * time.Hour, + ReportPullingDelay: 1 * time.Minute, + }, + err: fmt.Errorf("min_retry must be a non-negative duration"), + }, + { + name: "timeout cannot be empty", + ctrl: Controller{}, + obj: map[string]interface{}{ + "interval": "2h", + "pull_report": map[string]interface{}{ + "delay": "1m", + "min_retry": "2m", + }, + }, + expectedOutput: Controller{ + Interval: 2 * time.Hour, + ReportPullingDelay: 1 * time.Minute, + ReportMinRetryTime: 2 * time.Minute, + }, + err: fmt.Errorf("timeout must be a non-negative duration"), + }, + { + name: "storagePath cannot be empty", + ctrl: Controller{}, + obj: map[string]interface{}{ + "interval": "2h", + "pull_report": map[string]interface{}{ + "delay": "1m", + "min_retry": "2m", + "timeout": "5m", + }, + }, + expectedOutput: Controller{ + Interval: 2 * time.Hour, + ReportPullingDelay: 1 * time.Minute, + ReportMinRetryTime: 2 * time.Minute, + ReportPullingTimeout: 5 * time.Minute, + }, + err: fmt.Errorf("storagePath must point to a directory where snapshots can be stored"), + }, + { + name: "SCA interval must be valid duration", + ctrl: Controller{}, + obj: map[string]interface{}{ + "interval": "2h", + "pull_report": map[string]interface{}{ + "delay": "1m", + "min_retry": "2m", + "timeout": "5m", + }, + "storagePath": "test/path", + "ocm": map[string]interface{}{ + "scaInterval": "not-duration", + }, + }, + expectedOutput: Controller{ + Interval: 2 * time.Hour, + ReportPullingDelay: 1 * time.Minute, + ReportMinRetryTime: 2 * time.Minute, + ReportPullingTimeout: 5 * time.Minute, + StoragePath: "test/path", + }, + err: fmt.Errorf("OCM SCA interval must be a valid duration: time: invalid duration \"not-duration\""), + }, + { + name: "SCA interval must be valid duration", + ctrl: Controller{}, + obj: map[string]interface{}{ + "interval": "2h", + "pull_report": map[string]interface{}{ + "delay": "1m", + "min_retry": "2m", + "timeout": "5m", + }, + "storagePath": "test/path", + "ocm": map[string]interface{}{ + "clusterTransferInterval": "not-duration", + }, + }, + expectedOutput: Controller{ + Interval: 2 * time.Hour, + ReportPullingDelay: 1 * time.Minute, + ReportMinRetryTime: 2 * time.Minute, + ReportPullingTimeout: 5 * time.Minute, + StoragePath: "test/path", + }, + err: fmt.Errorf("OCM Cluster transfer interval must be a valid duration: time: invalid duration \"not-duration\""), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output, err := LoadConfig(tt.ctrl, tt.obj, ToController) + assert.Equal(t, tt.err, err) + assert.Equal(t, tt.expectedOutput, output) + }) + } +} diff --git a/pkg/config/types.go b/pkg/config/types.go index 994bc9745..5fdae0677 100644 --- a/pkg/config/types.go +++ b/pkg/config/types.go @@ -1,19 +1,27 @@ package config import ( - "fmt" "time" - - "k8s.io/klog/v2" ) -const defaultGatherPeriod = 2 * time.Hour +const ( + // defines default frequency of the data gathering + defaultGatherFrequency = 2 * time.Hour + // defines default frequency of the SCA download + defaultSCAFfrequency = 8 * time.Hour + // defines default frequency of the Cluster Transfer download + defaultClusterTransferFrequency = 12 * time.Hour +) // InsightsConfigurationSerialized is a type representing Insights // Operator configuration values in JSON/YAML and it is when decoding // the content of the "insights-config" config map. type InsightsConfigurationSerialized struct { - DataReporting DataReportingSerialized `json:"dataReporting"` + DataReporting DataReportingSerialized `json:"dataReporting"` + Alerting AlertingSerialized `json:"alerting,omitempty"` + SCA SCASerialized `json:"sca,omitempty"` + ClusterTransfer ClusterTransferSerialized `json:"clusterTransfer,omitempty"` + Proxy ProxySeriazlied `json:"proxy,omitempty"` } type DataReportingSerialized struct { @@ -27,11 +35,36 @@ type DataReportingSerialized struct { Obfuscation Obfuscation `json:"obfuscation,omitempty"` } +type AlertingSerialized struct { + Disabled string `json:"disabled,omitempty"` +} + +type SCASerialized struct { + Disabled string `json:"disabled,omitempty"` + Interval string `json:"interval,omitempty"` + Endpoint string `json:"endpoint,omitempty"` +} + +type ClusterTransferSerialized struct { + Interval string `json:"interval,omitempty"` + Endpoint string `json:"endpoint,omitempty"` +} + +type ProxySeriazlied struct { + HTTPProxy string `json:"httpProxy,omitempty"` + HTTPSProxy string `json:"httpsProxy,omitempty"` + NoProxy string `json:"noProxy,omitempty"` +} + // InsightsConfiguration is a type representing actual Insights // Operator configuration options and is used in the code base // to make the configuration available. type InsightsConfiguration struct { - DataReporting DataReporting + DataReporting DataReporting + Alerting Alerting + SCA SCA + ClusterTransfer ClusterTransfer + Proxy Proxy } // DataReporting is a type including all @@ -50,6 +83,34 @@ type DataReporting struct { Obfuscation Obfuscation } +// Alerting is a helper type for configuring Insights alerting +// options +type Alerting struct { + Disabled bool +} + +// ClusterTransfer is a helper type for configuring Insights +// cluster transfer (ownership) feature +type ClusterTransfer struct { + Interval time.Duration + Endpoint string +} + +// SCA is a helper type for configuring periodical download/check +// of the SimpleContentAcccess entitlements +type SCA struct { + Disabled bool + Interval time.Duration + Endpoint string +} + +// Proxy is a helper type for configuring connection proxy +type Proxy struct { + HTTPProxy string + HTTPSProxy string + NoProxy string +} + const ( Networking ObfuscationValue = "networking" WorkloadNames ObfuscationValue = "workload_names" @@ -58,47 +119,3 @@ const ( type ObfuscationValue string type Obfuscation []ObfuscationValue - -// ToConfig reads and pareses the actual serialized configuration from "InsightsConfigurationSerialized" -// and returns the "InsightsConfiguration". -func (i *InsightsConfigurationSerialized) ToConfig() *InsightsConfiguration { - ic := &InsightsConfiguration{ - DataReporting: DataReporting{ - UploadEndpoint: i.DataReporting.UploadEndpoint, - DownloadEndpoint: i.DataReporting.DownloadEndpoint, - DownloadEndpointTechPreview: i.DataReporting.DownloadEndpointTechPreview, - StoragePath: i.DataReporting.StoragePath, - ConditionalGathererEndpoint: i.DataReporting.ConditionalGathererEndpoint, - ProcessingStatusEndpoint: i.DataReporting.ProcessingStatusEndpoint, - Obfuscation: i.DataReporting.Obfuscation, - }, - } - if i.DataReporting.Interval != "" { - interval, err := time.ParseDuration(i.DataReporting.Interval) - if err != nil { - klog.Errorf("Cannot parse interval time duration: %v. Using default value %s", err, defaultGatherPeriod) - } - if interval <= 0 { - interval = defaultGatherPeriod - } - ic.DataReporting.Interval = interval - } - return ic -} - -func (i *InsightsConfiguration) String() string { - s := fmt.Sprintf(`upload_interval=%s, - upload_endpoint=%s, - storage_path=%s, - download_endpoint=%s, - conditional_gatherer_endpoint=%s, - obfuscation=%s`, - i.DataReporting.Interval, - i.DataReporting.UploadEndpoint, - i.DataReporting.StoragePath, - i.DataReporting.DownloadEndpoint, - i.DataReporting.ConditionalGathererEndpoint, - i.DataReporting.Obfuscation, - ) - return s -} diff --git a/pkg/controller/gather_commands.go b/pkg/controller/gather_commands.go index 8edd9e815..d43b5b110 100644 --- a/pkg/controller/gather_commands.go +++ b/pkg/controller/gather_commands.go @@ -87,7 +87,7 @@ func (g *GatherJob) Gather(ctx context.Context, kubeConfig, protoKubeConfig *res } }() - authorizer := clusterauthorizer.New(configObserver) + authorizer := clusterauthorizer.New(configObserver, configAggregator) // gatherConfigClient is configClient created from gatherKubeConfig, this name was used because configClient was already taken // this client is only used in insightsClient, it is created here @@ -170,7 +170,7 @@ func (g *GatherJob) GatherAndUpload(kubeConfig, protoKubeConfig *rest.Config) er // the recorder stores the collected data and we flush at the end. recdriver := diskrecorder.New(g.StoragePath) rec := recorder.New(recdriver, g.Interval, anonymizer) - authorizer := clusterauthorizer.New(configObserver) + authorizer := clusterauthorizer.New(configObserver, configAggregator) configClient, err := configv1client.NewForConfig(gatherKubeConfig) if err != nil { diff --git a/pkg/controller/operator.go b/pkg/controller/operator.go index 03b0957cd..9fff2b11a 100644 --- a/pkg/controller/operator.go +++ b/pkg/controller/operator.go @@ -168,7 +168,7 @@ func (s *Operator) Run(ctx context.Context, controller *controllercmd.Controller // the status controller initializes the cluster operator object and retrieves // the last sync time, if any was set - statusReporter := status.NewController(configClient.ConfigV1(), secretConfigObserver, + statusReporter := status.NewController(configClient.ConfigV1(), configAggregator, insightsDataGatherObserver, os.Getenv("POD_NAMESPACE")) var anonymizer *anonymization.Anonymizer @@ -192,7 +192,7 @@ func (s *Operator) Run(ctx context.Context, controller *controllercmd.Controller go rec.PeriodicallyPrune(ctx, statusReporter) } - authorizer := clusterauthorizer.New(secretConfigObserver) + authorizer := clusterauthorizer.New(secretConfigObserver, configAggregator) // gatherConfigClient is configClient created from gatherKubeConfig, this name was used because configClient was already taken // this client is only used in insightsClient, it is created here @@ -216,7 +216,7 @@ func (s *Operator) Run(ctx context.Context, controller *controllercmd.Controller operatorClient.InsightsOperators(), kubeClient) statusReporter.AddSources(periodicGather.Sources()...) } else { - reportRetriever := insightsreport.NewWithTechPreview(insightsClient, secretConfigObserver) + reportRetriever := insightsreport.NewWithTechPreview(insightsClient, configAggregator) periodicGather = periodic.NewWithTechPreview(reportRetriever, configAggregator, insightsDataGatherObserver, gatherers, kubeClient, insightClient.InsightsV1alpha1(), operatorClient.InsightsOperators(), dgInformer) statusReporter.AddSources(periodicGather.Sources()...) @@ -246,7 +246,7 @@ func (s *Operator) Run(ctx context.Context, controller *controllercmd.Controller // know any previous last reported time go uploader.Run(ctx) - reportGatherer := insightsreport.New(insightsClient, secretConfigObserver, uploader, operatorClient.InsightsOperators()) + reportGatherer := insightsreport.New(insightsClient, configAggregator, uploader, operatorClient.InsightsOperators()) statusReporter.AddSources(reportGatherer) go reportGatherer.Run(ctx) } @@ -256,17 +256,15 @@ func (s *Operator) Run(ctx context.Context, controller *controllercmd.Controller return fmt.Errorf("unable to set initial cluster status: %v", err) } - scaController := initiateSCAController(ctx, kubeClient, secretConfigObserver, insightsClient) - if scaController != nil { - statusReporter.AddSources(scaController) - go scaController.Run() - } + scaController := sca.New(ctx, kubeClient.CoreV1(), configAggregator, insightsClient) + statusReporter.AddSources(scaController) + go scaController.Run() - clusterTransferController := clustertransfer.New(ctx, kubeClient.CoreV1(), secretConfigObserver, insightsClient) + clusterTransferController := clustertransfer.New(ctx, kubeClient.CoreV1(), configAggregator, insightsClient) statusReporter.AddSources(clusterTransferController) go clusterTransferController.Run() - promRulesController := insights.NewPrometheusRulesController(secretConfigObserver, controller.KubeConfig) + promRulesController := insights.NewPrometheusRulesController(configAggregator, controller.KubeConfig) go promRulesController.Start(ctx) klog.Warning("started") @@ -304,12 +302,3 @@ func isRunning(kubeConfig *rest.Config) wait.ConditionWithContextFunc { return true, nil } } - -// initiateSCAController creates a new sca.Controller -func initiateSCAController(ctx context.Context, - kubeClient *kubernetes.Clientset, configObserver *configobserver.Controller, insightsClient *insightsclient.Client) *sca.Controller { - // SCA controller periodically checks and pull data from the OCM SCA API - // the data is exposed in the OpenShift API - scaController := sca.New(ctx, kubeClient.CoreV1(), configObserver, insightsClient) - return scaController -} diff --git a/pkg/controller/periodic/periodic.go b/pkg/controller/periodic/periodic.go index b01c02637..a741b3a7e 100644 --- a/pkg/controller/periodic/periodic.go +++ b/pkg/controller/periodic/periodic.go @@ -3,6 +3,7 @@ package periodic import ( "context" "fmt" + "reflect" "sort" "strconv" "strings" @@ -235,9 +236,10 @@ func (c *Controller) periodicTrigger(stopCh <-chan struct{}) { configCh, closeFn := c.configAggregator.ConfigChanged() defer closeFn() - interval := c.configAggregator.Config().DataReporting.Interval + config := c.configAggregator.Config() + interval := config.DataReporting.Interval klog.Infof("Gathering cluster info every %s", interval) - klog.Infof("Configuration is %v", c.configAggregator.Config().String()) + klog.Infof("Configuration is %v", config.String()) t := time.NewTicker(interval) for { select { @@ -245,15 +247,16 @@ func (c *Controller) periodicTrigger(stopCh <-chan struct{}) { t.Stop() return case <-configCh: - newInterval := c.configAggregator.Config().DataReporting.Interval - if newInterval == interval { + newConfig := c.configAggregator.Config() + if reflect.DeepEqual(config, newConfig) { continue } - interval = newInterval + config = newConfig + interval = config.DataReporting.Interval t.Reset(interval) klog.Infof("Gathering cluster info every %s", interval) - klog.Infof("Configuration is %v", c.configAggregator.Config().String()) + klog.Infof("Configuration is %v", config.String()) case <-t.C: if c.techPreview { c.GatherJob() diff --git a/pkg/controller/status/controller.go b/pkg/controller/status/controller.go index be0aa4fde..6d2ded5ae 100644 --- a/pkg/controller/status/controller.go +++ b/pkg/controller/status/controller.go @@ -54,9 +54,9 @@ type Controller struct { client configv1client.ConfigV1Interface - statusCh chan struct{} - secretConfigurator configobserver.Configurator - apiConfigurator configobserver.InsightsDataGatherObserver + statusCh chan struct{} + configurator configobserver.Interface + apiConfigurator configobserver.InsightsDataGatherObserver sources map[string]controllerstatus.StatusController reported Reported @@ -69,18 +69,18 @@ type Controller struct { // NewController creates a statusMessage controller, responsible for monitoring the operators statusMessage and updating its cluster statusMessage accordingly. func NewController(client configv1client.ConfigV1Interface, - secretConfigurator configobserver.Configurator, + configurator configobserver.Interface, apiConfigurator configobserver.InsightsDataGatherObserver, namespace string) *Controller { c := &Controller{ - name: "insights", - statusCh: make(chan struct{}, 1), - secretConfigurator: secretConfigurator, - apiConfigurator: apiConfigurator, - client: client, - namespace: namespace, - sources: make(map[string]controllerstatus.StatusController), - ctrlStatus: newControllerStatus(), + name: "insights", + statusCh: make(chan struct{}, 1), + configurator: configurator, + apiConfigurator: apiConfigurator, + client: client, + namespace: namespace, + sources: make(map[string]controllerstatus.StatusController), + ctrlStatus: newControllerStatus(), } return c } @@ -401,7 +401,7 @@ func (c *Controller) updateControllerConditionByReason(cs *conditions, func (c *Controller) checkDisabledGathering() { // disabled state only when it's disabled by config. It means that gathering will not happen - if !c.secretConfigurator.Config().Report { + if !c.configurator.Config().DataReporting.Enabled { c.ctrlStatus.setStatus(DisabledStatus, noTokenReason, reportingDisabledMsg) } diff --git a/pkg/controller/status/controller_test.go b/pkg/controller/status/controller_test.go index 7c6f7e4b8..d86d460b9 100644 --- a/pkg/controller/status/controller_test.go +++ b/pkg/controller/status/controller_test.go @@ -11,11 +11,9 @@ import ( configv1 "github.com/openshift/api/config/v1" configfake "github.com/openshift/client-go/config/clientset/versioned/fake" "github.com/openshift/insights-operator/pkg/config" - "github.com/openshift/insights-operator/pkg/config/configobserver" "github.com/openshift/insights-operator/pkg/utils" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - kubeclientfake "k8s.io/client-go/kubernetes/fake" ) func Test_Status_SaveInitialStart(t *testing.T) { @@ -68,14 +66,17 @@ func Test_Status_SaveInitialStart(t *testing.T) { if tt.clusterOperator != nil { operators = append(operators, tt.clusterOperator) } - kubeclientsetclient := kubeclientfake.NewSimpleClientset() client := configfake.NewSimpleClientset(operators...) ctrl := &Controller{ - name: "insights", - client: client.ConfigV1(), - secretConfigurator: configobserver.New(config.Controller{Report: true}, kubeclientsetclient), - ctrlStatus: newControllerStatus(), + name: "insights", + client: client.ConfigV1(), + configurator: config.NewMockConfigMapConfigurator(&config.InsightsConfiguration{ + DataReporting: config.DataReporting{ + Enabled: true, + }, + }), + ctrlStatus: newControllerStatus(), } err := ctrl.updateStatus(context.Background(), tt.initialRun) @@ -137,8 +138,12 @@ func Test_updatingConditionsInDisabledState(t *testing.T) { testController := Controller{ ctrlStatus: newControllerStatus(), // marking operator as disabled - secretConfigurator: config.NewMockSecretConfigurator(&config.Controller{Report: false}), - apiConfigurator: config.NewMockAPIConfigurator(nil), + configurator: config.NewMockConfigMapConfigurator(&config.InsightsConfiguration{ + DataReporting: config.DataReporting{ + Enabled: false, + }, + }), + apiConfigurator: config.NewMockAPIConfigurator(nil), } updatedCO := testController.merge(&testCO) // check that all the conditions are not touched except the disabled one @@ -206,8 +211,12 @@ func Test_updatingConditionsFromDegradedToDisabled(t *testing.T) { testController := Controller{ ctrlStatus: newControllerStatus(), // marking operator as disabled - secretConfigurator: config.NewMockSecretConfigurator(&config.Controller{Report: false}), - apiConfigurator: config.NewMockAPIConfigurator(nil), + configurator: config.NewMockConfigMapConfigurator(&config.InsightsConfiguration{ + DataReporting: config.DataReporting{ + Enabled: false, + }, + }), + apiConfigurator: config.NewMockAPIConfigurator(nil), } updatedCO := testController.merge(&testCO) // check that all conditions changed except the Progressing since it's still False diff --git a/pkg/insights/insightsreport/insightsreport.go b/pkg/insights/insightsreport/insightsreport.go index f9fbe9685..77d2dadb0 100644 --- a/pkg/insights/insightsreport/insightsreport.go +++ b/pkg/insights/insightsreport/insightsreport.go @@ -24,11 +24,16 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + minReportRetryTime = 30 * time.Second + reportDownloadTimeout = 5 * time.Minute +) + // Controller gathers the report from Smart Proxy type Controller struct { controllerstatus.StatusController - configurator configobserver.Configurator + configurator configobserver.Interface client insightsReportClient LastReport types.SmartProxyReport archiveUploadReporter <-chan struct{} @@ -53,7 +58,7 @@ var ( ) // New initializes and returns a Gatherer -func New(client *insightsclient.Client, configurator configobserver.Configurator, reporter InsightsReporter, insightsOperatorCLI operatorv1client.InsightsOperatorInterface) *Controller { +func New(client *insightsclient.Client, configurator configobserver.Interface, reporter InsightsReporter, insightsOperatorCLI operatorv1client.InsightsOperatorInterface) *Controller { return &Controller{ StatusController: controllerstatus.New("insightsreport"), configurator: configurator, @@ -63,7 +68,7 @@ func New(client *insightsclient.Client, configurator configobserver.Configurator } } -func NewWithTechPreview(client *insightsclient.Client, configurator configobserver.Configurator) *Controller { +func NewWithTechPreview(client *insightsclient.Client, configurator configobserver.Interface) *Controller { return &Controller{ StatusController: controllerstatus.New("insightsreport"), configurator: configurator, @@ -77,7 +82,7 @@ func NewWithTechPreview(client *insightsclient.Client, configurator configobserv func (c *Controller) PullReportTechpreview(insightsRequestID string) (*types.InsightsAnalysisReport, error) { klog.Info("Retrieving report from the insights-results-agregator service endpoint") config := c.configurator.Config() - reportEndpointTP := config.ReportEndpointTechPreview + reportEndpointTP := config.DataReporting.DownloadEndpointTechPreview if len(reportEndpointTP) == 0 { klog.V(4).Info("Not downloading report because the insights-results-agregator endpoint is not configured") @@ -130,7 +135,7 @@ func (c *Controller) PullReportTechpreview(insightsRequestID string) (*types.Ins func (c *Controller) PullSmartProxy() (bool, error) { klog.Info("Pulling report from smart-proxy") config := c.configurator.Config() - reportEndpoint := config.ReportEndpoint + reportEndpoint := config.DataReporting.DownloadEndpoint if len(reportEndpoint) == 0 { klog.V(4).Info("Not downloading report because Smart Proxy client is not properly configured: missing report endpoint") @@ -211,16 +216,11 @@ func (c *Controller) RetrieveReport() { configCh, cancelFn := c.configurator.ConfigChanged() defer cancelFn() - if config.ReportPullingTimeout == 0 { - klog.V(4).Info("Not downloading report because Smart Proxy client is not properly configured: missing polling timeout") - return - } - - delay := config.ReportPullingDelay + delay := config.DataReporting.ReportPullingDelay klog.V(4).Infof("Initial delay for pulling: %v", delay) startTime := time.Now() delayTimer := time.NewTimer(wait.Jitter(delay, 0.1)) - timeoutTimer := time.NewTimer(config.ReportPullingTimeout) + timeoutTimer := time.NewTimer(reportDownloadTimeout) firstPullDone := false retryCounter := 0 @@ -249,7 +249,7 @@ func (c *Controller) RetrieveReport() { }) return } - t := wait.Jitter(config.ReportMinRetryTime, 0.1) + t := wait.Jitter(minReportRetryTime, 0.1) klog.Infof("Reseting the delay timer to retry in %s again", t) delayTimer.Reset(t) retryCounter++ @@ -267,10 +267,10 @@ func (c *Controller) RetrieveReport() { // Update next deadline var nextTick time.Duration if firstPullDone { - newDeadline := iterationStart.Add(config.ReportMinRetryTime) + newDeadline := iterationStart.Add(minReportRetryTime) nextTick = wait.Jitter(time.Until(newDeadline), 0.3) } else { - newDeadline := iterationStart.Add(config.ReportPullingDelay) + newDeadline := iterationStart.Add(config.DataReporting.ReportPullingDelay) nextTick = wait.Jitter(time.Until(newDeadline), 0.1) } @@ -280,7 +280,7 @@ func (c *Controller) RetrieveReport() { delayTimer.Reset(nextTick) // Update pulling timeout - newTimeoutEnd := startTime.Add(config.ReportPullingTimeout) + newTimeoutEnd := startTime.Add(reportDownloadTimeout) if !timeoutTimer.Stop() { <-timeoutTimer.C } @@ -295,7 +295,7 @@ func (c *Controller) Run(ctx context.Context) { klog.V(2).Info("Starting report retriever") conf := c.configurator.Config() klog.V(2).Infof("Insights analysis reports will be downloaded from the %s endpoint with a delay of %s", - conf.ReportEndpoint, conf.ReportPullingDelay) + conf.DataReporting.DownloadEndpoint, conf.DataReporting.ReportPullingDelay) for { // always wait for new uploaded archive or insights-operator ends select { @@ -339,7 +339,7 @@ func (c *Controller) readInsightsReportTechPreview(report types.InsightsAnalysis case 4: healthStatus.critical++ } - if c.configurator.Config().DisableInsightsAlerts { + if c.configurator.Config().Alerting.Disabled { continue } insights.RecommendationCollector.SetClusterID(configv1.ClusterID(report.ClusterID)) @@ -378,7 +378,7 @@ func (c *Controller) readInsightsReport(report types.SmartProxyReport) ([]types. healthStatus.critical++ } - if c.configurator.Config().DisableInsightsAlerts { + if c.configurator.Config().Alerting.Disabled { continue } errorKeyStr, err := extractErrorKeyFromRuleData(rule) diff --git a/pkg/insights/insightsreport/insightsreport_test.go b/pkg/insights/insightsreport/insightsreport_test.go index f6229bb03..63827ff42 100644 --- a/pkg/insights/insightsreport/insightsreport_test.go +++ b/pkg/insights/insightsreport/insightsreport_test.go @@ -36,10 +36,9 @@ func Test_readInsightsReport(t *testing.T) { { name: "basic test with all rules enabled", testController: &Controller{ - configurator: config.NewMockSecretConfigurator(&config.Controller{ - DisableInsightsAlerts: false, - }), - client: &client, + + configurator: config.NewMockConfigMapConfigurator(&config.InsightsConfiguration{}), + client: &client, }, report: types.SmartProxyReport{ Data: []types.RuleWithContentResponse{ @@ -124,10 +123,8 @@ func Test_readInsightsReport(t *testing.T) { { name: "basic test with some rules disabled", testController: &Controller{ - configurator: config.NewMockSecretConfigurator(&config.Controller{ - DisableInsightsAlerts: false, - }), - client: &client, + configurator: config.NewMockConfigMapConfigurator(&config.InsightsConfiguration{}), + client: &client, }, report: types.SmartProxyReport{ Data: []types.RuleWithContentResponse{ @@ -200,8 +197,10 @@ func Test_readInsightsReport(t *testing.T) { { name: "Insights recommendations as alerts are disabled => no active recommendations", testController: &Controller{ - configurator: config.NewMockSecretConfigurator(&config.Controller{ - DisableInsightsAlerts: true, + configurator: config.NewMockConfigMapConfigurator(&config.InsightsConfiguration{ + Alerting: config.Alerting{ + Disabled: true, + }, }), client: &client, }, @@ -318,7 +317,7 @@ func TestPullReportTechpreview(t *testing.T) { name string report *types.InsightsAnalysisReport statusCode int - conf *config.Controller + conf config.InsightsConfiguration statusSummary controllerstatus.Summary mockClientErr error expectedErr error @@ -350,8 +349,10 @@ func TestPullReportTechpreview(t *testing.T) { }, }, statusCode: http.StatusOK, - conf: &config.Controller{ - ReportEndpointTechPreview: "non-empty-endpoint", + conf: config.InsightsConfiguration{ + DataReporting: config.DataReporting{ + DownloadEndpointTechPreview: "non-empty-endpoint", + }, }, statusSummary: controllerstatus.Summary{ Healthy: true, @@ -363,8 +364,10 @@ func TestPullReportTechpreview(t *testing.T) { name: "Empty report endpoint", report: nil, statusCode: 0, - conf: &config.Controller{ - ReportEndpointTechPreview: "", + conf: config.InsightsConfiguration{ + DataReporting: config.DataReporting{ + DownloadEndpointTechPreview: "", + }, }, statusSummary: controllerstatus.Summary{ Healthy: true, @@ -376,8 +379,10 @@ func TestPullReportTechpreview(t *testing.T) { name: "Insights Analysis Report not retrieved, because of error", report: nil, statusCode: 0, - conf: &config.Controller{ - ReportEndpointTechPreview: "non-empty-endpoint", + conf: config.InsightsConfiguration{ + DataReporting: config.DataReporting{ + DownloadEndpointTechPreview: "non-empty-endpoint", + }, }, statusSummary: controllerstatus.Summary{ Healthy: false, @@ -389,8 +394,10 @@ func TestPullReportTechpreview(t *testing.T) { name: "Insights Analysis Report not retrieved, because of HTTP 404 response", report: nil, statusCode: http.StatusNotFound, - conf: &config.Controller{ - ReportEndpointTechPreview: "non-empty-endpoint", + conf: config.InsightsConfiguration{ + DataReporting: config.DataReporting{ + DownloadEndpointTechPreview: "non-empty-endpoint", + }, }, statusSummary: controllerstatus.Summary{ Healthy: false, @@ -413,9 +420,7 @@ func TestPullReportTechpreview(t *testing.T) { }, err: tt.mockClientErr, }, - configurator: &config.MockSecretConfigurator{ - Conf: tt.conf, - }, + configurator: config.NewMockConfigMapConfigurator(&tt.conf), StatusController: controllerstatus.New("test-insightsreport"), } diff --git a/pkg/insights/prometheus_rules.go b/pkg/insights/prometheus_rules.go index 98f7688d5..3a3fa76fd 100644 --- a/pkg/insights/prometheus_rules.go +++ b/pkg/insights/prometheus_rules.go @@ -4,6 +4,7 @@ import ( "context" "github.com/openshift/insights-operator/pkg/config/configobserver" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/rest" "k8s.io/klog/v2" @@ -27,12 +28,12 @@ var ( // PrometheusRulesControllers listens to the configuration observer and // creates or removes the Insights Prometheus Rules definitions accordingly type PrometheusRulesController struct { - configurator configobserver.Configurator + configurator configobserver.Interface monitoringCS monitoringcli.Interface promRulesExist bool } -func NewPrometheusRulesController(configurator configobserver.Configurator, kubeConfig *rest.Config) PrometheusRulesController { +func NewPrometheusRulesController(configurator configobserver.Interface, kubeConfig *rest.Config) PrometheusRulesController { monitoringCS, err := monitoringcli.NewForConfig(kubeConfig) if err != nil { klog.Warningf("Unable create monitoring client: %v", err) @@ -62,7 +63,7 @@ func (p *PrometheusRulesController) Start(ctx context.Context) { // checkAlertsDisabled reads the actual config and either creates (if they don't exist) or removes (if they do exist) // the "insights-prometheus-rules" definition func (p *PrometheusRulesController) checkAlertsDisabled(ctx context.Context) { - disableInsightsAlerts := p.configurator.Config().DisableInsightsAlerts + disableInsightsAlerts := p.configurator.Config().Alerting.Disabled if disableInsightsAlerts && p.promRulesExist { err := p.removeInsightsAlerts(ctx) @@ -77,6 +78,10 @@ func (p *PrometheusRulesController) checkAlertsDisabled(ctx context.Context) { if !disableInsightsAlerts && !p.promRulesExist { err := p.createInsightsAlerts(ctx) if err != nil { + if errors.IsAlreadyExists(err) { + p.promRulesExist = true + return + } klog.Errorf("Failed to create Insights Prometheus rules definition: %v", err) return } diff --git a/pkg/insights/prometheus_rules_test.go b/pkg/insights/prometheus_rules_test.go index 9ffab3714..563fa92e4 100644 --- a/pkg/insights/prometheus_rules_test.go +++ b/pkg/insights/prometheus_rules_test.go @@ -42,8 +42,10 @@ func TestCheckAlertsDisabled(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mockConfigObserver := config.NewMockSecretConfigurator(&config.Controller{ - DisableInsightsAlerts: tt.alertsDisabled, + mockConfigObserver := config.NewMockConfigMapConfigurator(&config.InsightsConfiguration{ + Alerting: config.Alerting{ + Disabled: tt.alertsDisabled, + }, }) testMonCli := fakeMonCli.NewSimpleClientset() mockPromController := PrometheusRulesController{ diff --git a/pkg/ocm/clustertransfer/cluster_transfer.go b/pkg/ocm/clustertransfer/cluster_transfer.go index 3307829b6..f39a0fe5c 100644 --- a/pkg/ocm/clustertransfer/cluster_transfer.go +++ b/pkg/ocm/clustertransfer/cluster_transfer.go @@ -29,7 +29,7 @@ type Controller struct { controllerstatus.StatusController coreClient corev1client.CoreV1Interface ctx context.Context - configurator configobserver.Configurator + configurator configobserver.Interface client clusterTransferClient pullSecret *v1.Secret } @@ -41,7 +41,7 @@ type clusterTransferClient interface { // New creates new instance of the cluster transfer controller func New(ctx context.Context, coreClient corev1client.CoreV1Interface, - configurator configobserver.Configurator, + configurator configobserver.Interface, insightsClient clusterTransferClient) *Controller { return &Controller{ StatusController: controllerstatus.New(ControllerName), @@ -55,8 +55,8 @@ func New(ctx context.Context, // Run periodically queries the OCM API and update pull-secret accordingly func (c *Controller) Run() { cfg := c.configurator.Config() - endpoint := cfg.OCMConfig.ClusterTransferEndpoint - interval := cfg.OCMConfig.ClusterTransferInterval + endpoint := cfg.ClusterTransfer.Endpoint + interval := cfg.ClusterTransfer.Interval configCh, cancel := c.configurator.ConfigChanged() defer cancel() c.requestDataAndUpdateSecret(endpoint) @@ -66,8 +66,8 @@ func (c *Controller) Run() { c.requestDataAndUpdateSecret(endpoint) case <-configCh: cfg := c.configurator.Config() - interval = cfg.OCMConfig.ClusterTransferInterval - endpoint = cfg.OCMConfig.ClusterTransferEndpoint + interval = cfg.ClusterTransfer.Interval + endpoint = cfg.ClusterTransfer.Endpoint } } } @@ -75,7 +75,7 @@ func (c *Controller) Run() { // requestDataAndUpdateSecret queries the provided endpoint. If there is any data // in the response then check if a secret update is required, and if so, perform the update. func (c *Controller) requestDataAndUpdateSecret(endpoint string) { - klog.Infof("checking the availability of cluster transfer. Next check is in %s", c.configurator.Config().OCMConfig.ClusterTransferInterval) + klog.Infof("checking the availability of cluster transfer. Next check is in %s", c.configurator.Config().ClusterTransfer.Interval) data, err := c.requestClusterTransferWithExponentialBackoff(endpoint) if err != nil { msg := fmt.Sprintf("failed to pull cluster transfer: %v", err) @@ -209,11 +209,11 @@ func (c *Controller) updatePullSecret(newData []byte) error { // The exponential backoff is applied only for HTTP errors >= 500. func (c *Controller) requestClusterTransferWithExponentialBackoff(endpoint string) ([]byte, error) { bo := wait.Backoff{ - Duration: c.configurator.Config().OCMConfig.ClusterTransferInterval / 24, // 30 min as the first waiting + Duration: c.configurator.Config().ClusterTransfer.Interval / 24, // 30 min as the first waiting Factor: 2, Jitter: 0, Steps: ocm.FailureCountThreshold, - Cap: c.configurator.Config().OCMConfig.ClusterTransferInterval, + Cap: c.configurator.Config().ClusterTransfer.Interval, } var data []byte diff --git a/pkg/ocm/clustertransfer/cluster_transfer_test.go b/pkg/ocm/clustertransfer/cluster_transfer_test.go index 9576d9a24..b6363f7f7 100644 --- a/pkg/ocm/clustertransfer/cluster_transfer_test.go +++ b/pkg/ocm/clustertransfer/cluster_transfer_test.go @@ -125,11 +125,11 @@ func Test_ClusterTransfer_RequestDataAndUpdateSecret(t *testing.T) { t.Run(tt.name, func(t *testing.T) { kube := kubefake.NewSimpleClientset() coreClient := kube.CoreV1() - mockConfig := &config.MockSecretConfigurator{ - Conf: &config.Controller{ - OCMConfig: config.OCMConfig{ClusterTransferEndpoint: "/cluster_transfer"}, + mockConfig := config.NewMockConfigMapConfigurator(&config.InsightsConfiguration{ + ClusterTransfer: config.ClusterTransfer{ + Endpoint: "/cluster_transfer", }, - } + }) ctResponse, err := loadDataFromFile(tt.clusterTransferDataFilePath) assert.NoError(t, err) mockClient := &MockClusterTransferClient{data: string(ctResponse)} @@ -138,7 +138,7 @@ func Test_ClusterTransfer_RequestDataAndUpdateSecret(t *testing.T) { _, err = createPullSecretFromFile(coreClient, tt.pullSecretDataFilePath) assert.NoError(t, err) - ctController.requestDataAndUpdateSecret(mockConfig.Conf.OCMConfig.ClusterTransferEndpoint) + ctController.requestDataAndUpdateSecret(mockConfig.Config().ClusterTransfer.Endpoint) summary, ok := ctController.CurrentStatus() assert.True(t, ok, "unexpected summary") assert.Equal(t, tt.expectedSummary.Operation, summary.Operation) diff --git a/pkg/ocm/sca/sca.go b/pkg/ocm/sca/sca.go index e3e3e559b..20ad170eb 100644 --- a/pkg/ocm/sca/sca.go +++ b/pkg/ocm/sca/sca.go @@ -35,7 +35,7 @@ type Controller struct { controllerstatus.StatusController coreClient corev1client.CoreV1Interface ctx context.Context - configurator configobserver.Configurator + configurator configobserver.Interface client *insightsclient.Client } @@ -48,7 +48,7 @@ type Response struct { } // New creates new instance -func New(ctx context.Context, coreClient corev1client.CoreV1Interface, configurator configobserver.Configurator, +func New(ctx context.Context, coreClient corev1client.CoreV1Interface, configurator configobserver.Interface, insightsClient *insightsclient.Client) *Controller { return &Controller{ StatusController: controllerstatus.New(ControllerName), @@ -62,9 +62,9 @@ func New(ctx context.Context, coreClient corev1client.CoreV1Interface, configura // Run periodically queries the OCM API and update corresponding secret accordingly func (c *Controller) Run() { cfg := c.configurator.Config() - endpoint := cfg.OCMConfig.SCAEndpoint - interval := cfg.OCMConfig.SCAInterval - disabled := cfg.OCMConfig.SCADisabled + endpoint := cfg.SCA.Endpoint + interval := cfg.SCA.Interval + disabled := cfg.SCA.Disabled configCh, cancel := c.configurator.ConfigChanged() defer cancel() if !disabled { @@ -88,16 +88,16 @@ func (c *Controller) Run() { } case <-configCh: cfg := c.configurator.Config() - interval = cfg.OCMConfig.SCAInterval - endpoint = cfg.OCMConfig.SCAEndpoint - disabled = cfg.OCMConfig.SCADisabled + interval = cfg.SCA.Interval + endpoint = cfg.SCA.Endpoint + disabled = cfg.SCA.Disabled } } } func (c *Controller) requestDataAndCheckSecret(endpoint string) { - klog.Infof("Pulling SCA certificates from %s. Next check is in %s", c.configurator.Config().OCMConfig.SCAEndpoint, - c.configurator.Config().OCMConfig.SCAInterval) + klog.Infof("Pulling SCA certificates from %s. Next check is in %s", c.configurator.Config().SCA.Endpoint, + c.configurator.Config().SCA.Interval) data, err := c.requestSCAWithExpBackoff(endpoint) if err != nil { httpErr, ok := err.(insightsclient.HttpError) @@ -210,11 +210,11 @@ func (c *Controller) updateSecret(s *v1.Secret, ocmData *Response) (*v1.Secret, // The exponential backoff is applied only for HTTP errors >= 500. func (c *Controller) requestSCAWithExpBackoff(endpoint string) ([]byte, error) { bo := wait.Backoff{ - Duration: c.configurator.Config().OCMConfig.SCAInterval / 32, // 15 min by default + Duration: c.configurator.Config().SCA.Interval / 32, // 15 min by default Factor: 2, Jitter: 0, Steps: ocm.FailureCountThreshold, - Cap: c.configurator.Config().OCMConfig.SCAInterval, + Cap: c.configurator.Config().SCA.Interval, } var data []byte err := wait.ExponentialBackoff(bo, func() (bool, error) {