Skip to content

Commit 1ac8280

Browse files
[receiver/tlscheck] Move Target Validation to scrape & Implement Scrape Errors (#40341)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This PR moves target validation from `config.go` to `scraper.go`. This causes a change in behavior. Previously, the receiver would crash if a target failed validation. With this change, the receiver will log an error message and increment the `otelcol_scraper_errored_metric_points` metric. <!--Describe what testing was performed and which tests were added.--> #### Testing I added tests for the new validation function. --------- Co-authored-by: Antoine Toulme <[email protected]>
1 parent af8553f commit 1ac8280

File tree

6 files changed

+228
-160
lines changed

6 files changed

+228
-160
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: tlscheckreceiver
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Do not crash on target validation & implement better scrape errors
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [40341]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]

receiver/tlscheckreceiver/README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ receivers:
4343
4444
This component does not provide hostname, validity period, path, or CRL / OCSP verification on the certificate.
4545
46+
## Certificate File Validation
47+
48+
If a certificate file specified in the configuration does not exist or is unable to be opened, an error will be logged on each scrape cycle and the `otelcol_scraper_errored_metric_points` metric will be incremented. If you would like to monitor for the existence of specific certificate files on disk, consider using the [File Stats receiver](../filestatsreceiver/README.md).
49+
4650
## Metrics
4751

4852
Details about the metrics produced by this receiver can be found in [metadata.yaml](./metadata.yaml).

receiver/tlscheckreceiver/config.go

Lines changed: 0 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,6 @@ package tlscheckreceiver // import "github.com/open-telemetry/opentelemetry-coll
55

66
import (
77
"errors"
8-
"fmt"
9-
"net"
10-
"os"
11-
"path/filepath"
12-
"strconv"
13-
"strings"
148

159
"go.opentelemetry.io/collector/config/confignet"
1610
"go.opentelemetry.io/collector/scraper/scraperhelper"
@@ -42,77 +36,13 @@ type Config struct {
4236
_ struct{}
4337
}
4438

45-
func validatePort(port string) error {
46-
portNum, err := strconv.Atoi(port)
47-
if err != nil {
48-
return fmt.Errorf("provided port is not a number: %s", port)
49-
}
50-
if portNum < 1 || portNum > 65535 {
51-
return fmt.Errorf("provided port is out of valid range (1-65535): %d", portNum)
52-
}
53-
return nil
54-
}
55-
5639
func validateTarget(ct *CertificateTarget) error {
57-
// Check that exactly one of endpoint or file_path is specified
5840
if ct.Endpoint != "" && ct.FilePath != "" {
5941
return errors.New("cannot specify both endpoint and file_path")
6042
}
6143
if ct.Endpoint == "" && ct.FilePath == "" {
6244
return errors.New("must specify either endpoint or file_path")
6345
}
64-
65-
// Validate endpoint if specified
66-
if ct.Endpoint != "" {
67-
if strings.Contains(ct.Endpoint, "://") {
68-
return fmt.Errorf("endpoint contains a scheme, which is not allowed: %s", ct.Endpoint)
69-
}
70-
71-
_, port, err := net.SplitHostPort(ct.Endpoint)
72-
if err != nil {
73-
return fmt.Errorf("%s: %w", errInvalidEndpoint.Error(), err)
74-
}
75-
76-
if err := validatePort(port); err != nil {
77-
return fmt.Errorf("%s: %w", errInvalidEndpoint.Error(), err)
78-
}
79-
}
80-
81-
// Validate file path if specified
82-
if ct.FilePath != "" {
83-
// Clean the path to handle different path separators
84-
cleanPath := filepath.Clean(ct.FilePath)
85-
86-
// Check if the path is absolute
87-
if !filepath.IsAbs(cleanPath) {
88-
return fmt.Errorf("file path must be absolute: %s", ct.FilePath)
89-
}
90-
91-
// Check if path exists and is a regular file
92-
fileInfo, err := os.Stat(cleanPath)
93-
if err != nil {
94-
if os.IsNotExist(err) {
95-
return fmt.Errorf("certificate file does not exist: %s", ct.FilePath)
96-
}
97-
return fmt.Errorf("error accessing certificate file %s: %w", ct.FilePath, err)
98-
}
99-
100-
// check if it is a directory
101-
if fileInfo.IsDir() {
102-
return fmt.Errorf("path is a directory, not a file: %s", cleanPath)
103-
}
104-
105-
// Check if it's a regular file (not a directory or special file)
106-
if !fileInfo.Mode().IsRegular() {
107-
return fmt.Errorf("certificate path is not a regular file: %s", ct.FilePath)
108-
}
109-
110-
// Check if file is readable
111-
if _, err := os.ReadFile(cleanPath); err != nil {
112-
return fmt.Errorf("certificate file is not readable: %s", ct.FilePath)
113-
}
114-
}
115-
11646
return nil
11747
}
11848

receiver/tlscheckreceiver/config_test.go

Lines changed: 0 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ package tlscheckreceiver // import "github.com/open-telemetry/opentelemetry-coll
55

66
import (
77
"errors"
8-
"fmt"
98
"os"
10-
"path/filepath"
119
"testing"
1210
"time"
1311

@@ -92,34 +90,6 @@ func TestValidate(t *testing.T) {
9290
},
9391
expectedErr: nil,
9492
},
95-
{
96-
desc: "invalid endpoint",
97-
cfg: &Config{
98-
Targets: []*CertificateTarget{
99-
{
100-
TCPAddrConfig: confignet.TCPAddrConfig{
101-
Endpoint: "bad-endpoint:12efg",
102-
},
103-
},
104-
},
105-
ControllerConfig: scraperhelper.NewDefaultControllerConfig(),
106-
},
107-
expectedErr: fmt.Errorf("%w: provided port is not a number: 12efg", errInvalidEndpoint),
108-
},
109-
{
110-
desc: "endpoint with scheme",
111-
cfg: &Config{
112-
Targets: []*CertificateTarget{
113-
{
114-
TCPAddrConfig: confignet.TCPAddrConfig{
115-
Endpoint: "https://example.com:443",
116-
},
117-
},
118-
},
119-
ControllerConfig: scraperhelper.NewDefaultControllerConfig(),
120-
},
121-
expectedErr: errors.New("endpoint contains a scheme, which is not allowed: https://example.com:443"),
122-
},
12393
{
12494
desc: "both endpoint and file path",
12595
cfg: &Config{
@@ -135,56 +105,6 @@ func TestValidate(t *testing.T) {
135105
},
136106
expectedErr: errors.New("cannot specify both endpoint and file_path"),
137107
},
138-
{
139-
desc: "relative file path",
140-
cfg: &Config{
141-
Targets: []*CertificateTarget{
142-
{
143-
FilePath: "cert.pem",
144-
},
145-
},
146-
ControllerConfig: scraperhelper.NewDefaultControllerConfig(),
147-
},
148-
expectedErr: errors.New("file path must be absolute: cert.pem"),
149-
},
150-
{
151-
desc: "nonexistent file",
152-
cfg: &Config{
153-
Targets: []*CertificateTarget{
154-
{
155-
FilePath: filepath.Join(tmpDir, "nonexistent.pem"),
156-
},
157-
},
158-
ControllerConfig: scraperhelper.NewDefaultControllerConfig(),
159-
},
160-
expectedErr: errors.New("certificate file does not exist"),
161-
},
162-
{
163-
desc: "directory instead of file",
164-
cfg: &Config{
165-
Targets: []*CertificateTarget{
166-
{
167-
FilePath: tmpDir,
168-
},
169-
},
170-
ControllerConfig: scraperhelper.NewDefaultControllerConfig(),
171-
},
172-
expectedErr: fmt.Errorf("path is a directory, not a file: %s", tmpDir),
173-
},
174-
{
175-
desc: "port out of range",
176-
cfg: &Config{
177-
Targets: []*CertificateTarget{
178-
{
179-
TCPAddrConfig: confignet.TCPAddrConfig{
180-
Endpoint: "example.com:67000",
181-
},
182-
},
183-
},
184-
ControllerConfig: scraperhelper.NewDefaultControllerConfig(),
185-
},
186-
expectedErr: fmt.Errorf("%w: provided port is out of valid range (1-65535): 67000", errInvalidEndpoint),
187-
},
188108
}
189109

190110
for _, tc := range testCases {

0 commit comments

Comments
 (0)