Skip to content

Commit d784310

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

2 files changed

Lines changed: 152 additions & 9 deletions

File tree

‎main.go‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"os/exec"
1919
"path/filepath"
2020
"reflect"
21+
"runtime"
2122
"slices"
2223
"strconv"
2324
"strings"
@@ -565,8 +566,14 @@ commandFinished:
565566
if resources != nil {
566567
result.Resources = resources
567568
}
568-
logPaths, logTail, logErr := readCustodianLogArtifacts(req.OutputDir, custodianOutputTailBytes)
569-
result.LogPaths = logPaths
569+
var logTail string
570+
var logErr error
571+
if req.LogTailDuringRun || err != nil || runCtx.Err() != nil || resourcesErr != nil {
572+
logPaths, tail, readErr := readCustodianLogArtifacts(req.OutputDir, custodianOutputTailBytes)
573+
result.LogPaths = logPaths
574+
logTail = tail
575+
logErr = readErr
576+
}
570577

571578
if err != nil {
572579
result.Err = fmt.Errorf("custodian execution failed: %w", err)
@@ -728,7 +735,7 @@ func (e *CommandCustodianExecutor) runAWSEndpointDiagnostics(ctx context.Context
728735
e.Logger.Error("AWS endpoint diagnostics configuration is invalid", "check_name", req.Check.Name, "resource", req.Check.Resource, "error", endpointErr)
729736
return endpointErr
730737
}
731-
if !knownResource {
738+
if !knownResource && len(endpoints) == 0 {
732739
err := fmt.Errorf("resource service is not mapped for AWS endpoint diagnostics: %s", req.Check.Resource)
733740
e.Logger.Error("AWS endpoint diagnostics failed before custodian execution", "check_name", req.Check.Name, "resource", req.Check.Resource, "error", err)
734741
return err
@@ -738,6 +745,9 @@ func (e *CommandCustodianExecutor) runAWSEndpointDiagnostics(ctx context.Context
738745
e.Logger.Error("AWS endpoint diagnostics failed before custodian execution", "check_name", req.Check.Name, "resource", req.Check.Resource, "aws_regions", req.AWSRegions, "error", err)
739746
return err
740747
}
748+
if !knownResource {
749+
e.Logger.Warn("AWS endpoint diagnostics will use only configured endpoints because resource service is not mapped", "check_name", req.Check.Name, "resource", req.Check.Resource)
750+
}
741751

742752
var diagnosticsErr error
743753
for _, endpoint := range endpoints {
@@ -781,18 +791,17 @@ func awsEndpointHostsForCheck(resource string, regions []string) ([]string, bool
781791

782792
func awsDiagnosticEndpointsForCheck(resource string, regions []string, configuredEndpoints []string) ([]networkDiagnosticEndpoint, bool, error) {
783793
services, knownResource := awsServicesForResource(resource)
784-
if !knownResource {
785-
return nil, false, nil
786-
}
787-
788794
endpoints := make([]networkDiagnosticEndpoint, 0)
789795
for _, endpointValue := range configuredEndpoints {
790796
endpoint, err := parseNetworkDiagnosticEndpoint(endpointValue)
791797
if err != nil {
792-
return nil, true, err
798+
return nil, knownResource, err
793799
}
794800
endpoints = append(endpoints, endpoint)
795801
}
802+
if !knownResource {
803+
return compactUniqueNetworkDiagnosticEndpoints(endpoints), false, nil
804+
}
796805

797806
diagnosticRegions := make([]string, 0, len(regions))
798807
for _, region := range regions {
@@ -854,7 +863,7 @@ func parseNetworkDiagnosticEndpoint(value string) (networkDiagnosticEndpoint, er
854863

855864
func networkDiagnosticEndpointSource(host string) string {
856865
host = strings.ToLower(strings.TrimSpace(host))
857-
if strings.Contains(host, ".vpce.amazonaws.com") || strings.Contains(host, ".vpce.amazonaws.com.cn") {
866+
if strings.HasSuffix(host, ".vpce.amazonaws.com") || strings.HasSuffix(host, ".vpce.amazonaws.com.cn") {
858867
return "aws-vpc-endpoint"
859868
}
860869
return "configured"
@@ -976,6 +985,15 @@ func (e *CommandCustodianExecutor) logCustodianRunLogTail(outputDir, checkName,
976985
}
977986

978987
func custodianProcessSockets(pid int) ([]string, error) {
988+
if runtime.GOOS != "linux" {
989+
return []string{}, nil
990+
}
991+
if _, err := os.Stat("/proc"); err != nil {
992+
if errors.Is(err, fs.ErrNotExist) {
993+
return []string{}, nil
994+
}
995+
return nil, err
996+
}
979997
inodes, err := processSocketInodes(pid)
980998
if err != nil {
981999
return nil, err
@@ -1059,6 +1077,16 @@ func decodeProcNetAddress(value string, ipv6 bool) string {
10591077
return fmt.Sprintf("%d.%d.%d.%d:%d", ipBytes[3], ipBytes[2], ipBytes[1], ipBytes[0], port)
10601078
}
10611079
}
1080+
if ipv6 && len(parts[0]) == 32 {
1081+
ipBytes, err := hex.DecodeString(parts[0])
1082+
if err == nil && len(ipBytes) == net.IPv6len {
1083+
for index := 0; index < len(ipBytes); index += 4 {
1084+
ipBytes[index], ipBytes[index+3] = ipBytes[index+3], ipBytes[index]
1085+
ipBytes[index+1], ipBytes[index+2] = ipBytes[index+2], ipBytes[index+1]
1086+
}
1087+
return net.JoinHostPort(net.IP(ipBytes).String(), strconv.FormatUint(port, 10))
1088+
}
1089+
}
10621090
return fmt.Sprintf("%s:%d", parts[0], port)
10631091
}
10641092

‎main_test.go‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,56 @@ touch "$EXECUTED_FILE"
566566
}
567567
})
568568

569+
t.Run("network diagnostics allow configured endpoints for unmapped resources", func(t *testing.T) {
570+
stubNetworkDiagnostics(
571+
t,
572+
func(ctx context.Context, host string) ([]string, error) {
573+
return []string{"10.0.0.10"}, nil
574+
},
575+
func(ctx context.Context, endpoint networkDiagnosticEndpoint) (tlsProbeResult, error) {
576+
if endpoint.Host != "vpce-123.example.eu-west-1.vpce.amazonaws.com" {
577+
t.Fatalf("unexpected endpoint host: %s", endpoint.Host)
578+
}
579+
return tlsProbeResult{RemoteAddr: "10.0.0.10:443", TLSVersion: "TLS1.3"}, nil
580+
},
581+
)
582+
583+
script := `#!/bin/sh
584+
set -eu
585+
out=""
586+
while [ "$#" -gt 0 ]; do
587+
if [ "$1" = "-s" ]; then
588+
out="$2"
589+
shift 2
590+
continue
591+
fi
592+
shift
593+
done
594+
mkdir -p "$out/test-policy"
595+
printf '[]' > "$out/test-policy/resources.json"
596+
`
597+
binary := writeExecutableScript(t, script)
598+
executor := &CommandCustodianExecutor{Logger: hclog.NewNullLogger()}
599+
600+
result := executor.Execute(context.Background(), CustodianExecutionRequest{
601+
BinaryPath: binary,
602+
Check: CustodianCheck{
603+
Name: "test-policy",
604+
Resource: "aws.future-resource",
605+
Provider: "aws",
606+
RawPolicy: map[string]interface{}{"name": "test-policy", "resource": "aws.future-resource"},
607+
},
608+
Timeout: 5 * time.Second,
609+
OutputDir: filepath.Join(t.TempDir(), "out"),
610+
NetworkDiagnostics: true,
611+
NetworkDiagnosticEndpoints: []string{"vpce-123.example.eu-west-1.vpce.amazonaws.com"},
612+
})
613+
614+
if result.Err != nil {
615+
t.Fatalf("expected successful execution, got error: %v", result.Err)
616+
}
617+
})
618+
569619
t.Run("passes debug and verbose args", func(t *testing.T) {
570620
argsFile := filepath.Join(t.TempDir(), "args.txt")
571621
t.Setenv("ARGS_FILE", argsFile)
@@ -755,6 +805,45 @@ exit 3
755805
}
756806
})
757807

808+
t.Run("does not read custodian log artifacts on success by default", func(t *testing.T) {
809+
script := `#!/bin/sh
810+
set -eu
811+
out=""
812+
while [ "$#" -gt 0 ]; do
813+
if [ "$1" = "-s" ]; then
814+
out="$2"
815+
shift 2
816+
continue
817+
fi
818+
shift
819+
done
820+
mkdir -p "$out/test-policy/us-east-1/test-policy"
821+
printf 'success log detail\n' > "$out/test-policy/us-east-1/test-policy/custodian-run.log"
822+
printf '[]' > "$out/test-policy/resources.json"
823+
`
824+
binary := writeExecutableScript(t, script)
825+
executor := &CommandCustodianExecutor{Logger: hclog.NewNullLogger()}
826+
827+
result := executor.Execute(context.Background(), CustodianExecutionRequest{
828+
BinaryPath: binary,
829+
Check: CustodianCheck{
830+
Name: "test-policy",
831+
Resource: "aws.backup-vault",
832+
Provider: "aws",
833+
RawPolicy: map[string]interface{}{"name": "test-policy", "resource": "aws.backup-vault"},
834+
},
835+
Timeout: 5 * time.Second,
836+
OutputDir: filepath.Join(t.TempDir(), "out"),
837+
})
838+
839+
if result.Err != nil {
840+
t.Fatalf("expected successful execution, got error: %v", result.Err)
841+
}
842+
if len(result.LogPaths) != 0 {
843+
t.Fatalf("expected successful execution not to walk log artifacts by default, got %#v", result.LogPaths)
844+
}
845+
})
846+
758847
t.Run("strips plugin-only policy fields before custodian execution", func(t *testing.T) {
759848
script := `#!/bin/sh
760849
set -eu
@@ -891,6 +980,15 @@ func TestDiagnosticHelpers(t *testing.T) {
891980
}
892981
})
893982

983+
t.Run("uses strict vpc endpoint suffix classification", func(t *testing.T) {
984+
if got := networkDiagnosticEndpointSource("evil.vpce.amazonaws.com.attacker.com"); got != "configured" {
985+
t.Fatalf("expected attacker suffix not to be classified as vpc endpoint, got %s", got)
986+
}
987+
if got := networkDiagnosticEndpointSource("vpce-123.backup.eu-west-1.vpce.amazonaws.com"); got != "aws-vpc-endpoint" {
988+
t.Fatalf("expected vpc endpoint classification, got %s", got)
989+
}
990+
})
991+
894992
t.Run("rejects invalid configured endpoint ports", func(t *testing.T) {
895993
_, _, err := awsDiagnosticEndpointsForCheck("aws.backup-vault", nil, []string{"vpce-123.backup.eu-west-1.vpce.amazonaws.com:not-a-port"})
896994
if err == nil {
@@ -908,6 +1006,19 @@ func TestDiagnosticHelpers(t *testing.T) {
9081006
}
9091007
})
9101008

1009+
t.Run("allows configured endpoints for unknown resource types", func(t *testing.T) {
1010+
endpoints, known, err := awsDiagnosticEndpointsForCheck("aws.not-yet-mapped", []string{"eu-west-1"}, []string{"vpce-123.example.eu-west-1.vpce.amazonaws.com"})
1011+
if err != nil {
1012+
t.Fatalf("unexpected endpoint parse error: %v", err)
1013+
}
1014+
if known {
1015+
t.Fatalf("expected resource to remain unknown")
1016+
}
1017+
if len(endpoints) != 1 || endpoints[0].Host != "vpce-123.example.eu-west-1.vpce.amazonaws.com" {
1018+
t.Fatalf("expected configured endpoint for unknown resource, got %#v", endpoints)
1019+
}
1020+
})
1021+
9111022
t.Run("maps known policy resource services", func(t *testing.T) {
9121023
resources := []string{
9131024
"aws.app-elb",
@@ -955,6 +1066,10 @@ func TestDiagnosticHelpers(t *testing.T) {
9551066
if got != "127.0.0.1:443" {
9561067
t.Fatalf("unexpected decoded address: %s", got)
9571068
}
1069+
got = decodeProcNetAddress("00000000000000000000000001000000:01BB", true)
1070+
if got != "[::1]:443" {
1071+
t.Fatalf("unexpected decoded IPv6 address: %s", got)
1072+
}
9581073
})
9591074

9601075
t.Run("upserts environment values", func(t *testing.T) {

0 commit comments

Comments
 (0)