Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adressing PR comments - https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/5052 #3

Merged
merged 2 commits into from
Sep 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions receiver/prometheusreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"strings"
"time"

config_util "github.com/prometheus/common/config"
commonconfig "github.com/prometheus/common/config"
promconfig "github.com/prometheus/prometheus/config"
"github.com/prometheus/prometheus/discovery/file"
"github.com/prometheus/prometheus/discovery/kubernetes"
Expand Down Expand Up @@ -58,7 +58,7 @@ type Config struct {
var _ config.Receiver = (*Config)(nil)
var _ config.Unmarshallable = (*Config)(nil)

func checkFileExists(fn string) error {
func checkFile(fn string) error {
// Nothing set, nothing to error on.
if fn == "" {
return nil
Expand All @@ -67,11 +67,11 @@ func checkFileExists(fn string) error {
return err
}

func checkTLSConfig(tlsConfig config_util.TLSConfig) error {
if err := checkFileExists(tlsConfig.CertFile); err != nil {
func checkTLSConfig(tlsConfig commonconfig.TLSConfig) error {
if err := checkFile(tlsConfig.CertFile); err != nil {
return fmt.Errorf("error checking client cert file %q - %v", tlsConfig.CertFile, err)
}
if err := checkFileExists(tlsConfig.KeyFile); err != nil {
if err := checkFile(tlsConfig.KeyFile); err != nil {
return fmt.Errorf("error checking client key file %q - %v", tlsConfig.KeyFile, err)
}
if len(tlsConfig.CertFile) > 0 && len(tlsConfig.KeyFile) == 0 {
Expand All @@ -83,6 +83,8 @@ func checkTLSConfig(tlsConfig config_util.TLSConfig) error {
return nil
}

// Method to exercise the prometheus file discovery behaviour to ensure there are no errors
// - reference https://github.com/prometheus/prometheus/blob/c0c22ed04200a8d24d1d5719f605c85710f0d008/discovery/file/file.go#L372
func checkSDFile(filename string) error {
fd, err := os.Open(filename)
if err != nil {
Expand Down Expand Up @@ -163,7 +165,7 @@ func (cfg *Config) Validate() error {
}

if sc.HTTPClientConfig.Authorization != nil {
if err := checkFileExists(sc.HTTPClientConfig.Authorization.CredentialsFile); err != nil {
if err := checkFile(sc.HTTPClientConfig.Authorization.CredentialsFile); err != nil {
return fmt.Errorf("error checking authorization credentials file %q - %s", sc.HTTPClientConfig.Authorization.CredentialsFile, err)
}
}
Expand Down
14 changes: 7 additions & 7 deletions receiver/prometheusreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func TestNonExistentAuthCredentialsFile(t *testing.T) {
require.Equal(t, wantErrMsg, gotErrMsg)
}

func TestTLSConfigCertFileExists(t *testing.T) {
func TestTLSConfigNonExistentCertFile(t *testing.T) {
factories, err := componenttest.NopFactories()
assert.NoError(t, err)

Expand All @@ -198,7 +198,7 @@ func TestTLSConfigCertFileExists(t *testing.T) {
require.Equal(t, wantErrMsg, gotErrMsg)
}

func TestTLSConfigKeyFileExists(t *testing.T) {
func TestTLSConfigNonExistentKeyFile(t *testing.T) {
factories, err := componenttest.NopFactories()
assert.NoError(t, err)

Expand All @@ -216,7 +216,7 @@ func TestTLSConfigKeyFileExists(t *testing.T) {
require.Equal(t, wantErrMsg, gotErrMsg)
}

func TestTLSConfigCertFileWithNoKeyFile(t *testing.T) {
func TestTLSConfigCertFileWithoutKeyFile(t *testing.T) {
factories, err := componenttest.NopFactories()
assert.NoError(t, err)

Expand All @@ -234,7 +234,7 @@ func TestTLSConfigCertFileWithNoKeyFile(t *testing.T) {
require.Equal(t, wantErrMsg, gotErrMsg)
}

func TestTLSConfigKeyFileWithNoCertFile(t *testing.T) {
func TestTLSConfigKeyFileWithoutCertFile(t *testing.T) {
factories, err := componenttest.NopFactories()
assert.NoError(t, err)

Expand All @@ -252,7 +252,7 @@ func TestTLSConfigKeyFileWithNoCertFile(t *testing.T) {
require.Equal(t, wantErrMsg, gotErrMsg)
}

func TestKubernetesSDConfig(t *testing.T) {
func TestKubernetesSDConfigWithoutKeyFile(t *testing.T) {
factories, err := componenttest.NopFactories()
assert.NoError(t, err)

Expand Down Expand Up @@ -282,7 +282,7 @@ func TestFileSDConfigJsonNilTargetGroup(t *testing.T) {
err = cfg.Validate()
require.NotNil(t, err, "Expected a non-nil error")

wantErrMsg := `receiver "prometheus" has invalid configuration: checking SD file "./testdata/dummy-sd-config.json": nil target group item found (index 1)`
wantErrMsg := `receiver "prometheus" has invalid configuration: checking SD file "./testdata/sd-config-with-null-target-group.json": nil target group item found (index 1)`

gotErrMsg := err.Error()
require.Equal(t, wantErrMsg, gotErrMsg)
Expand All @@ -300,7 +300,7 @@ func TestFileSDConfigYamlNilTargetGroup(t *testing.T) {
err = cfg.Validate()
require.NotNil(t, err, "Expected a non-nil error")

wantErrMsg := `receiver "prometheus" has invalid configuration: checking SD file "./testdata/dummy-sd-config.yaml": nil target group item found (index 1)`
wantErrMsg := `receiver "prometheus" has invalid configuration: checking SD file "./testdata/sd-config-with-null-target-group.yaml": nil target group item found (index 1)`

gotErrMsg := err.Error()
require.Equal(t, wantErrMsg, gotErrMsg)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ receivers:
- job_name: 'demo'
scrape_interval: 5s
file_sd_configs:
- files: ["./testdata/dummy-sd-config.json"]
- files: ["./testdata/sd-config-with-null-target-group.json"]

processors:
nop:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ receivers:
- job_name: 'demo'
scrape_interval: 5s
file_sd_configs:
- files: ["./testdata/dummy-sd-config.yaml"]
- files: ["./testdata/sd-config-with-null-target-group.yaml"]

processors:
nop:
Expand Down