Skip to content

Commit a4e9f40

Browse files
committed
fix: copilot issues
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
1 parent 76f9458 commit a4e9f40

2 files changed

Lines changed: 75 additions & 7 deletions

File tree

main.go

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -788,6 +788,9 @@ func (e *CommandCustodianExecutor) runAWSEndpointDiagnostics(ctx context.Context
788788

789789
var diagnosticsErr error
790790
for _, endpoint := range endpoints {
791+
if err := ctx.Err(); err != nil {
792+
return errors.Join(diagnosticsErr, err)
793+
}
791794
lookupCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
792795
lookupStarted := time.Now()
793796
ips, err := lookupHost(lookupCtx, endpoint.Host)
@@ -895,9 +898,13 @@ func parseNetworkDiagnosticEndpoint(value string) (networkDiagnosticEndpoint, er
895898
if port == "" {
896899
port = "443"
897900
}
898-
if _, err := strconv.Atoi(port); err != nil {
901+
portNumber, err := strconv.Atoi(port)
902+
if err != nil {
899903
return networkDiagnosticEndpoint{}, fmt.Errorf("network diagnostic endpoint %q has invalid port %q: %w", original, port, err)
900904
}
905+
if portNumber < 1 || portNumber > 65535 {
906+
return networkDiagnosticEndpoint{}, fmt.Errorf("network diagnostic endpoint %q has invalid port %q: must be between 1 and 65535", original, port)
907+
}
901908
return networkDiagnosticEndpoint{
902909
Host: strings.ToLower(host),
903910
Port: port,
@@ -934,19 +941,30 @@ type tlsProbeResult struct {
934941
}
935942

936943
func defaultTLSProbeEndpoint(ctx context.Context, endpoint networkDiagnosticEndpoint) (tlsProbeResult, error) {
937-
dialer := &net.Dialer{Timeout: 5 * time.Second}
944+
if err := ctx.Err(); err != nil {
945+
return tlsProbeResult{}, err
946+
}
947+
netDialer := &net.Dialer{Timeout: 5 * time.Second}
938948
if deadline, ok := ctx.Deadline(); ok {
939-
if remaining := time.Until(deadline); remaining > 0 && remaining < dialer.Timeout {
940-
dialer.Timeout = remaining
949+
if remaining := time.Until(deadline); remaining > 0 && remaining < netDialer.Timeout {
950+
netDialer.Timeout = remaining
941951
}
942952
}
943-
conn, err := tls.DialWithDialer(dialer, "tcp", net.JoinHostPort(endpoint.Host, endpoint.Port), &tls.Config{ServerName: endpoint.ServerName, MinVersion: tls.VersionTLS12})
953+
tlsDialer := &tls.Dialer{
954+
NetDialer: netDialer,
955+
Config: &tls.Config{ServerName: endpoint.ServerName, MinVersion: tls.VersionTLS12},
956+
}
957+
conn, err := tlsDialer.DialContext(ctx, "tcp", net.JoinHostPort(endpoint.Host, endpoint.Port))
944958
if err != nil {
945959
return tlsProbeResult{}, err
946960
}
947961
defer conn.Close()
948962

949-
state := conn.ConnectionState()
963+
tlsConn, ok := conn.(*tls.Conn)
964+
if !ok {
965+
return tlsProbeResult{}, errors.New("TLS dial returned non-TLS connection")
966+
}
967+
state := tlsConn.ConnectionState()
950968
return tlsProbeResult{
951969
RemoteAddr: conn.RemoteAddr().String(),
952970
TLSVersion: tlsVersionString(state.Version),

main_test.go

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,35 @@ printf '[]' > "$out/test-policy/resources.json"
616616
}
617617
})
618618

619+
t.Run("network diagnostics stop when context is canceled", func(t *testing.T) {
620+
stubNetworkDiagnostics(
621+
t,
622+
func(ctx context.Context, host string) ([]string, error) {
623+
t.Fatalf("did not expect DNS probe after context cancellation, got host %s", host)
624+
return nil, nil
625+
},
626+
func(ctx context.Context, endpoint networkDiagnosticEndpoint) (tlsProbeResult, error) {
627+
t.Fatalf("did not expect TLS probe after context cancellation, got endpoint %#v", endpoint)
628+
return tlsProbeResult{}, nil
629+
},
630+
)
631+
executor := &CommandCustodianExecutor{Logger: hclog.NewNullLogger()}
632+
ctx, cancel := context.WithCancel(context.Background())
633+
cancel()
634+
635+
err := executor.runAWSEndpointDiagnostics(ctx, CustodianExecutionRequest{
636+
Check: CustodianCheck{
637+
Name: "test-policy",
638+
Resource: "aws.backup-vault",
639+
Provider: "aws",
640+
},
641+
NetworkDiagnosticEndpoints: []string{"vpce-123.backup.eu-west-1.vpce.amazonaws.com"},
642+
})
643+
if !errors.Is(err, context.Canceled) {
644+
t.Fatalf("expected context canceled error, got %v", err)
645+
}
646+
})
647+
619648
t.Run("network diagnostics skip service probes when regions are not concrete", func(t *testing.T) {
620649
stubNetworkDiagnostics(
621650
t,
@@ -1094,6 +1123,27 @@ func TestDiagnosticHelpers(t *testing.T) {
10941123
if err == nil {
10951124
t.Fatalf("expected invalid endpoint port error")
10961125
}
1126+
for _, endpoint := range []string{
1127+
"vpce-123.backup.eu-west-1.vpce.amazonaws.com:0",
1128+
"vpce-123.backup.eu-west-1.vpce.amazonaws.com:65536",
1129+
} {
1130+
if _, _, err := awsDiagnosticEndpointsForCheck("aws.backup-vault", nil, []string{endpoint}); err == nil {
1131+
t.Fatalf("expected invalid endpoint port error for %s", endpoint)
1132+
}
1133+
}
1134+
})
1135+
1136+
t.Run("tls probe returns immediately when context is canceled", func(t *testing.T) {
1137+
ctx, cancel := context.WithCancel(context.Background())
1138+
cancel()
1139+
_, err := defaultTLSProbeEndpoint(ctx, networkDiagnosticEndpoint{
1140+
Host: "example.invalid",
1141+
Port: "443",
1142+
ServerName: "example.invalid",
1143+
})
1144+
if !errors.Is(err, context.Canceled) {
1145+
t.Fatalf("expected context canceled error, got %v", err)
1146+
}
10971147
})
10981148

10991149
t.Run("reports unknown resource types", func(t *testing.T) {
@@ -1801,7 +1851,7 @@ func TestAWSResourceExplorerURL(t *testing.T) {
18011851
}
18021852
})
18031853

1804-
t.Run("logs common non arn skipped link generation at debug", func(t *testing.T) {
1854+
t.Run("does not warn for common non arn skipped link generation", func(t *testing.T) {
18051855
var logs bytes.Buffer
18061856
logger := hclog.New(&hclog.LoggerOptions{
18071857
Name: "test",

0 commit comments

Comments
 (0)