Fix prometheusreceiver TA/scrape_config validation logic (#30181)

This PR is a fork from #30135 where the author did not respond to the
comments. I applied all suggestions because as a maintainer is my
responsibility to ensure we fix bugs and merge PRs.

Thanks @Aneurysm9 for the first commit.

---------

Signed-off-by: Anthony J Mirabella <[email protected]>
Signed-off-by: Bogdan Drutu <[email protected]>
Co-authored-by: Anthony J Mirabella <[email protected]>

Author: bogdandrutu

.chloggen/fix_promTACfgValidation.yaml

@@ -0,0 +1,6 @@
+change_type: 'bug_fix'
+component: 'prometheusreceiver'
+note: Fix configuration validation to allow specification of Target Allocator configuration without providing scrape configurations
+issues: [30135]
+subtext:
+change_logs: []

receiver/prometheusreceiver/config.go

@@ -95,11 +95,13 @@ func checkTLSConfig(tlsConfig commonconfig.TLSConfig) error {
 // Validate checks the receiver configuration is valid.
 func (cfg *Config) Validate() error {
 	promConfig := cfg.PrometheusConfig
-	if promConfig != nil {
-		err := cfg.validatePromConfig(promConfig)
-		if err != nil {
-			return err
-		}
+	if (promConfig == nil || len(promConfig.ScrapeConfigs) == 0) && cfg.TargetAllocator == nil {
+		return errors.New("no Prometheus scrape_configs or target_allocator set")
+	}
+
+	err := validatePromConfig(promConfig)
+	if err != nil {
+		return err
 	}
 
 	if cfg.TargetAllocator != nil {
@@ -111,11 +113,10 @@ func (cfg *Config) Validate() error {
 	return nil
 }
 
-func (cfg *Config) validatePromConfig(promConfig *promconfig.Config) error {
-	if len(promConfig.ScrapeConfigs) == 0 && cfg.TargetAllocator == nil {
-		return errors.New("no Prometheus scrape_configs or target_allocator set")
+func validatePromConfig(promConfig *promconfig.Config) error {
+	if promConfig == nil {
+		return nil
 	}
-
 	// Reject features that Prometheus supports but that the receiver doesn't support:
 	// See:
 	// * https://github.com/open-telemetry/opentelemetry-collector/issues/3863
@@ -142,7 +143,7 @@ func (cfg *Config) validatePromConfig(promConfig *promconfig.Config) error {
 		return fmt.Errorf("unsupported features:\n\t%s", strings.Join(unsupportedFeatures, "\n\t"))
 	}
 
-	for _, sc := range cfg.PrometheusConfig.ScrapeConfigs {
+	for _, sc := range promConfig.ScrapeConfigs {
 		if sc.HTTPClientConfig.Authorization != nil {
 			if err := checkFile(sc.HTTPClientConfig.Authorization.CredentialsFile); err != nil {
 				return fmt.Errorf("error checking authorization credentials file %q: %w", sc.HTTPClientConfig.Authorization.CredentialsFile, err)

receiver/prometheusreceiver/config_test.go

@@ -66,6 +66,7 @@ func TestLoadTargetAllocatorConfig(t *testing.T) {
 	sub, err := cm.Sub(component.NewIDWithName(metadata.Type, "").String())
 	require.NoError(t, err)
 	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, component.ValidateConfig(cfg))
 
 	r0 := cfg.(*Config)
 	assert.NotNil(t, r0.PrometheusConfig)
@@ -77,6 +78,7 @@ func TestLoadTargetAllocatorConfig(t *testing.T) {
 	require.NoError(t, err)
 	cfg = factory.CreateDefaultConfig()
 	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, component.ValidateConfig(cfg))
 
 	r1 := cfg.(*Config)
 	assert.NotNil(t, r0.PrometheusConfig)
@@ -92,6 +94,7 @@ func TestLoadTargetAllocatorConfig(t *testing.T) {
 	require.NoError(t, err)
 	cfg = factory.CreateDefaultConfig()
 	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, component.ValidateConfig(cfg))
 
 	r2 := cfg.(*Config)
 	assert.Equal(t, 1, len(r2.PrometheusConfig.ScrapeConfigs))
@@ -110,6 +113,36 @@ func TestLoadConfigFailsOnUnknownSection(t *testing.T) {
 	require.Error(t, component.UnmarshalConfig(sub, cfg))
 }
 
+func TestLoadConfigFailsOnNoPrometheusOrTAConfig(t *testing.T) {
+	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "invalid-config-prometheus-non-existent-scrape-config.yaml"))
+	require.NoError(t, err)
+	factory := NewFactory()
+
+	cfg := factory.CreateDefaultConfig()
+	sub, err := cm.Sub(component.NewIDWithName(metadata.Type, "").String())
+	require.NoError(t, err)
+	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.ErrorContains(t, component.ValidateConfig(cfg), "no Prometheus scrape_configs or target_allocator set")
+
+	cfg = factory.CreateDefaultConfig()
+	sub, err = cm.Sub(component.NewIDWithName(metadata.Type, "withConfigAndTA").String())
+	require.NoError(t, err)
+	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, component.ValidateConfig(cfg))
+
+	cfg = factory.CreateDefaultConfig()
+	sub, err = cm.Sub(component.NewIDWithName(metadata.Type, "withOnlyTA").String())
+	require.NoError(t, err)
+	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, component.ValidateConfig(cfg))
+
+	cfg = factory.CreateDefaultConfig()
+	sub, err = cm.Sub(component.NewIDWithName(metadata.Type, "withOnlyScrape").String())
+	require.NoError(t, err)
+	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, component.ValidateConfig(cfg))
+}
+
 // As one of the config parameters is consuming prometheus
 // configuration as a subkey, ensure that invalid configuration
 // within the subkey will also raise an error.

receiver/prometheusreceiver/testdata/invalid-config-prometheus-non-existent-scrape-config.yaml

@@ -0,0 +1,19 @@
+prometheus:
+prometheus/withConfigAndTA:
+  target_allocator:
+    endpoint: http://localhost:8080
+    interval: 30s
+    collector_id: collector-1
+  config:
+    global:
+      scrape_interval: 30s
+prometheus/withOnlyTA:
+  target_allocator:
+    endpoint: http://localhost:8080
+    interval: 30s
+    collector_id: collector-1
+prometheus/withOnlyScrape:
+  config:
+    scrape_configs:
+      - job_name: 'demo'
+        scrape_interval: 5s