From 3d37148696742ed1a2cda7c23357834b1bc803f5 Mon Sep 17 00:00:00 2001 From: Rahul Varma Date: Wed, 11 Aug 2021 10:34:44 -0700 Subject: [PATCH] Added validation in webhook to validate prometheus config --- .../opentelemetrycollector_webhook.go | 10 +++++++ pkg/collector/reconcile/configmap.go | 3 +- pkg/collector/reconcile/configmap_test.go | 6 +--- pkg/collector/reconcile/deployment.go | 4 --- pkg/collector/reconcile/helper.go | 29 ------------------- pkg/collector/reconcile/service.go | 4 --- .../adapters/config_to_prom_config.go | 9 +++++- .../adapters/config_to_prom_config_test.go | 16 ++-------- 8 files changed, 23 insertions(+), 58 deletions(-) delete mode 100644 pkg/collector/reconcile/helper.go diff --git a/api/v1alpha1/opentelemetrycollector_webhook.go b/api/v1alpha1/opentelemetrycollector_webhook.go index 2ed41c6bef..361e0c7004 100644 --- a/api/v1alpha1/opentelemetrycollector_webhook.go +++ b/api/v1alpha1/opentelemetrycollector_webhook.go @@ -21,6 +21,8 @@ import ( ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" + + ta "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator/adapters" ) // log is for logging in this package. @@ -96,5 +98,13 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error { return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the target allocation deployment", r.Spec.Mode) } + // validate Prometheus config for target allocation + if r.Spec.TargetAllocator.Enabled { + _, err := ta.ConfigToPromConfig(r.Spec.Config) + if err != nil { + return fmt.Errorf("the OpenTelemetry Spec configuration is incorrect, %s", err) + } + } + return nil } diff --git a/pkg/collector/reconcile/configmap.go b/pkg/collector/reconcile/configmap.go index 9bb78e9461..cb57d9c45e 100644 --- a/pkg/collector/reconcile/configmap.go +++ b/pkg/collector/reconcile/configmap.go @@ -30,6 +30,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/pkg/collector" "github.com/open-telemetry/opentelemetry-operator/pkg/naming" "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator" + ta "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator/adapters" ) // +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;create;update;patch;delete @@ -84,7 +85,7 @@ func desiredTAConfigMap(params Params) (corev1.ConfigMap, error) { labels := targetallocator.Labels(params.Instance) labels["app.kubernetes.io/name"] = name - promConfig, err := GetPromConfig(params) + promConfig, err := ta.ConfigToPromConfig(params.Instance.Spec.Config) if err != nil { return corev1.ConfigMap{}, err } diff --git a/pkg/collector/reconcile/configmap_test.go b/pkg/collector/reconcile/configmap_test.go index 15d71e56d5..d9319c6560 100644 --- a/pkg/collector/reconcile/configmap_test.go +++ b/pkg/collector/reconcile/configmap_test.go @@ -28,7 +28,6 @@ import ( "github.com/open-telemetry/opentelemetry-operator/api/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" ta "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator/adapters" ) @@ -210,10 +209,7 @@ func TestExpectedConfigMap(t *testing.T) { assert.True(t, exists) assert.Equal(t, instanceUID, actual.OwnerReferences[0].UID) - config, err := adapters.ConfigFromString(params().Instance.Spec.Config) - assert.NoError(t, err) - - parmConfig, err := ta.ConfigToPromConfig(config) + parmConfig, err := ta.ConfigToPromConfig(params().Instance.Spec.Config) assert.NoError(t, err) taConfig := make(map[interface{}]interface{}) diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index a46b1d2410..9a4cba39db 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/pkg/collector/reconcile/deployment.go @@ -38,10 +38,6 @@ func Deployments(ctx context.Context, params Params) error { } if params.Instance.Spec.TargetAllocator.Enabled { - _, err := GetPromConfig(params) - if err != nil { - return fmt.Errorf("failed to parse Prometheus config: %v", err) - } desired = append(desired, targetallocator.Deployment(params.Config, params.Log, params.Instance)) } diff --git a/pkg/collector/reconcile/helper.go b/pkg/collector/reconcile/helper.go deleted file mode 100644 index 06ea1db6a4..0000000000 --- a/pkg/collector/reconcile/helper.go +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package reconcile - -import ( - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" - ta "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator/adapters" -) - -func GetPromConfig(params Params) (map[interface{}]interface{}, error) { - config, err := adapters.ConfigFromString(params.Instance.Spec.Config) - if err != nil { - return nil, err - } - - return ta.ConfigToPromConfig(config) -} diff --git a/pkg/collector/reconcile/service.go b/pkg/collector/reconcile/service.go index 8863bc73cd..d50760470a 100644 --- a/pkg/collector/reconcile/service.go +++ b/pkg/collector/reconcile/service.go @@ -51,10 +51,6 @@ func Services(ctx context.Context, params Params) error { } if params.Instance.Spec.TargetAllocator.Enabled { - _, err := GetPromConfig(params) - if err != nil { - return fmt.Errorf("failed to parse Prometheus config: %v", err) - } desired = append(desired, desiredTAService(params)) } diff --git a/pkg/targetallocator/adapters/config_to_prom_config.go b/pkg/targetallocator/adapters/config_to_prom_config.go index eb34f2dfc8..b4ba5f4d59 100644 --- a/pkg/targetallocator/adapters/config_to_prom_config.go +++ b/pkg/targetallocator/adapters/config_to_prom_config.go @@ -16,6 +16,8 @@ package adapters import ( "fmt" + + "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" ) func errorNoComponent(component string) error { @@ -27,7 +29,12 @@ func errorNotAMap(component string) error { } // ConfigToPromConfig converts the incoming configuration object into a the Prometheus receiver config. -func ConfigToPromConfig(config map[interface{}]interface{}) (map[interface{}]interface{}, error) { +func ConfigToPromConfig(cfg string) (map[interface{}]interface{}, error) { + config, err := adapters.ConfigFromString(cfg) + if err != nil { + return nil, err + } + receiversProperty, ok := config["receivers"] if !ok { return nil, errorNoComponent("receivers") diff --git a/pkg/targetallocator/adapters/config_to_prom_config_test.go b/pkg/targetallocator/adapters/config_to_prom_config_test.go index 208c9989c7..ba5cc2978a 100644 --- a/pkg/targetallocator/adapters/config_to_prom_config_test.go +++ b/pkg/targetallocator/adapters/config_to_prom_config_test.go @@ -20,9 +20,7 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" ta "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator/adapters" ) @@ -49,13 +47,8 @@ func TestExtractPromConfigFromConfig(t *testing.T) { }, } - // prepare - config, err := adapters.ConfigFromString(configStr) - require.NoError(t, err) - require.NotEmpty(t, config) - // test - promConfig, err := ta.ConfigToPromConfig(config) + promConfig, err := ta.ConfigToPromConfig(configStr) assert.NoError(t, err) // verify @@ -76,13 +69,8 @@ func TestExtractPromConfigFromNullConfig(t *testing.T) { endpoint: 0.0.0.0:15268 ` - // prepare - config, err := adapters.ConfigFromString(configStr) - require.NoError(t, err) - require.NotEmpty(t, config) - // test - promConfig, err := ta.ConfigToPromConfig(config) + promConfig, err := ta.ConfigToPromConfig(configStr) assert.Equal(t, err, fmt.Errorf("%s property in the configuration doesn't contain valid %s", "prometheusConfig", "prometheusConfig")) // verify