Skip to content

Commit 2871c56

Browse files
committed
worked on PR comments
1 parent ef99d2a commit 2871c56

File tree

8 files changed

+39
-24
lines changed

8 files changed

+39
-24
lines changed

api/grpc/mpi/v1/command.pb.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/grpc/mpi/v1/common.pb.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/grpc/mpi/v1/files.pb.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/collector/otel_collector_plugin_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -820,9 +820,13 @@ func Test_setProxyWithBasicAuth(t *testing.T) {
820820
httpProxy := os.Getenv("HTTPS_PROXY")
821821
assert.Equal(t, proxyURL, httpProxy)
822822

823+
logBuf := &bytes.Buffer{}
824+
stub.StubLoggerWith(logBuf)
823825
// Test missing username/password
824826
proxyMissing := &config.Proxy{URL: "http://localhost:8080"}
825-
setProxyWithBasicAuth(ctx, proxyMissing, u) // Should not panic
827+
setProxyWithBasicAuth(ctx, proxyMissing, u)
828+
helpers.ValidateLog(t, "Unable to configure OTLP exporter proxy, "+
829+
"username or password missing for basic auth", logBuf)
826830
}
827831

828832
//nolint:contextcheck // Can not update the "OTelConfig" function definition

internal/config/defaults.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ const (
3333
DefCommandTLSCaKey = ""
3434
DefCommandTLSSkipVerifyKey = false
3535
DefCommandTLServerNameKey = ""
36-
DefCommandServerProxyTimeoutKey = 0
3736
DefCommandServerProxyURlKey = ""
3837
DefCommandServerProxyNoProxyKey = ""
3938
DefCommandServerProxyAuthMethodKey = ""
@@ -45,6 +44,7 @@ const (
4544
DefCommandServerProxyTLSCaKey = ""
4645
DefCommandServerProxyTLSSkipVerifyKey = false
4746
DefCommandServerProxyTLServerNameKey = ""
47+
DefCommandServerProxyTimeoutKey = 10 * time.Second
4848

4949
DefAuxiliaryCommandServerHostKey = ""
5050
DefAuxiliaryCommandServerPortKey = 0

internal/config/types.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,16 +340,15 @@ type (
340340
MonitoringFrequency time.Duration `yaml:"monitoring_frequency" mapstructure:"monitoring_frequency"`
341341
}
342342

343-
//nolint:govet // Tried different sequences but not able to fix the lint issue.
344343
Proxy struct {
345344
TLS *TLSConfig `yaml:"tls,omitempty" mapstructure:"tls"`
346-
Timeout time.Duration `yaml:"timeout" mapstructure:"timeout"`
347345
URL string `yaml:"url" mapstructure:"url"`
348346
NoProxy string `yaml:"no_proxy,omitempty" mapstructure:"no_proxy"`
349347
AuthMethod string `yaml:"auth_method,omitempty" mapstructure:"auth_method"`
350348
Username string `yaml:"username,omitempty" mapstructure:"username"`
351349
Password string `yaml:"password,omitempty" mapstructure:"password"`
352350
Token string `yaml:"token,omitempty" mapstructure:"token"`
351+
Timeout time.Duration `yaml:"timeout" mapstructure:"timeout"`
353352
}
354353
)
355354

internal/grpc/proxy_dialer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func buildProxyTLSConfig(proxyConf *config.Proxy) (*tls.Config, error) {
7272
return tlsConf, nil
7373
}
7474

75-
//nolint:lll //This needs to be in a single line.
75+
//nolint:lll //This needs to be in a single line else getting gofumpt or compiler error.
7676
func dialToProxyTLS(ctx context.Context, proxyURL *url.URL, tlsConf *tls.Config, timeout time.Duration) (net.Conn, error) {
7777
dialer := &net.Dialer{Timeout: timeout}
7878
tlsDialer := &tls.Dialer{

internal/grpc/proxy_dialer_test.go

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -65,17 +65,15 @@ func TestDialViaHTTPProxy_RealProxy(t *testing.T) {
6565
t.Errorf("failed to write to tunnel: %v", err)
6666
}
6767
buf := make([]byte, 128)
68-
if deadlineErr := conn.SetReadDeadline(time.Now().Add(2 * time.Second)); deadlineErr != nil {
69-
// Optionally log
70-
t.Logf("Failed to set read deadline: %v", deadlineErr)
71-
}
68+
require.NoError(t, conn.SetReadDeadline(time.Now().Add(2*time.Second)), "failed to set read deadline")
69+
7270
_, err = conn.Read(buf)
73-
if err != nil && err != context.DeadlineExceeded && !isTimeout(err) {
71+
if err != nil && err != context.DeadlineExceeded && !os.IsTimeout(err) {
7472
t.Errorf("failed to read from tunnel: %v", err)
7573
}
7674
}
7775

78-
//nolint:noctx //No need for ctx in test cases.
76+
//nolint:noctx,revive //No need for ctx in test cases.
7977
func TestDialViaHTTPProxy_BearerTokenHeader(t *testing.T) {
8078
ln, err := net.Listen("tcp", "127.0.0.1:0")
8179
require.NoError(t, err, "failed to listen")
@@ -91,8 +89,21 @@ func TestDialViaHTTPProxy_BearerTokenHeader(t *testing.T) {
9189
defer conn.Close()
9290
reader := bufio.NewReader(conn)
9391
headerLines := readHeaders(reader)
94-
if hasBearerHeader(headerLines, "testtoken") {
95-
close(done)
92+
93+
if !hasBearerHeader(headerLines, "testtoken") {
94+
_, writeErr := conn.Write([]byte("HTTP/1.1 407 Proxy Authentication Required\r\n" +
95+
"Proxy-Authenticate: Bearer realm=\"nginx-agent\"\r\n\r\n"))
96+
if writeErr != nil {
97+
t.Errorf("Warning: mock proxy failed to write 407 response: %v", writeErr)
98+
}
99+
t.Errorf("Proxy-Authorization Bearer header with token 'testtoken' was not found in received request")
100+
101+
return
102+
}
103+
104+
_, writeErr := conn.Write([]byte("HTTP/1.1 200 Connection established\r\n\r\n"))
105+
if writeErr != nil {
106+
t.Errorf("mock proxy failed to write 200 OK response: %v", writeErr)
96107
return
97108
}
98109
close(done)
@@ -106,21 +117,23 @@ func TestDialViaHTTPProxy_BearerTokenHeader(t *testing.T) {
106117
}
107118
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
108119
defer cancel()
109-
_, _ = DialViaHTTPProxy(ctx, proxyConf, "example.com:443")
120+
_, err = DialViaHTTPProxy(ctx, proxyConf, "example.com:443")
121+
122+
if err != nil && err != context.DeadlineExceeded && !os.IsTimeout(err) {
123+
require.NoError(t, err, "DialViaHTTPProxy returned an unexpected non-timeout error")
124+
}
110125

111126
select {
112127
case <-done:
113128
// success
114129
case <-time.After(1 * time.Second):
115-
t.Errorf("Proxy-Authorization Bearer header was not sent")
130+
if err == context.DeadlineExceeded {
131+
t.Fatalf("Test timed out (DialViaHTTPProxy context deadline exceeded): %v", err)
132+
}
133+
t.Fatalf("Test timed out: Proxy-Authorization Bearer header was not sent or verified within %v", err)
116134
}
117135
}
118136

119-
func isTimeout(err error) bool {
120-
nerr, ok := err.(net.Error)
121-
return ok && nerr.Timeout()
122-
}
123-
124137
func readHeaders(reader *bufio.Reader) []string {
125138
var headerLines []string
126139
for {
@@ -166,7 +179,6 @@ func TestDialViaHTTPProxy_MissingCertKey(t *testing.T) {
166179
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
167180
defer cancel()
168181
_, err := DialViaHTTPProxy(ctx, proxyConf, "example.com:443")
169-
// No assert needed: just covers the branch
170182
require.Error(t, err, "expected error for missing cert")
171183
}
172184

0 commit comments

Comments
 (0)