diff --git a/pkg/httpclient/httpclient_test.go b/pkg/httpclient/httpclient_test.go index 1620458f..93b14fc8 100644 --- a/pkg/httpclient/httpclient_test.go +++ b/pkg/httpclient/httpclient_test.go @@ -59,3 +59,35 @@ func TestNew_AllowsRegularEndpoints(t *testing.T) { t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusOK) } } + +// The single-address metadata entries (/128 and /32) must not widen: the +// neighbors of each stay allowed. IPv4 link-local is deliberately a whole /16, +// so 169.254.169.253 is blocked, not allowed (#227). +func TestBlockMetadata_NeighborsOfSingleAddressEntries(t *testing.T) { + blocked := []string{ + "169.254.169.253:80", // inside the link-local /16 + "169.254.169.255:80", + } + for _, addr := range blocked { + t.Run("blocked "+addr, func(t *testing.T) { + if err := blockMetadata("tcp", addr, nil); err == nil { + t.Fatalf("%s must be blocked", addr) + } + }) + } + + allowed := []string{ + "[fd00:ec2::253]:80", "[fd00:ec2::255]:80", // AWS IPv6 IMDS /128 + "[fd00:ec2::22]:80", "[fd00:ec2::24]:80", // AWS Pod Identity /128 + "[fd20:ce::253]:80", "[fd20:ce::255]:80", "[fd20:ce:0:1::254]:80", // GCP IPv6 /128 + "168.63.129.15:80", "168.63.129.17:80", // Azure WireServer /32 + "100.100.100.199:80", "100.100.100.201:80", // Alibaba IMDS /32 + } + for _, addr := range allowed { + t.Run("allowed "+addr, func(t *testing.T) { + if err := blockMetadata("tcp", addr, nil); err != nil { + t.Fatalf("%s must stay allowed: %v", addr, err) + } + }) + } +} diff --git a/pkg/recfilter/sizing.go b/pkg/recfilter/sizing.go index dcbb18bb..ba47f80f 100644 --- a/pkg/recfilter/sizing.go +++ b/pkg/recfilter/sizing.go @@ -373,7 +373,7 @@ func targetCoverageInputsValid(rec common.Recommendation, targetPct float64) boo } func dropInvalidTargetCoverageInput(rec common.Recommendation, targetPct float64, logf Logf) string { - logf.printf("WARNING: --target-coverage=%v has invalid or out-of-range inputs for %s/%s/%s (avg=%v, existing=%v%%); dropped recommendation\n", + logf.printf("WARNING: --target-coverage=%.1f%% has invalid or out-of-range inputs for %s/%s/%s (avg=%v, existing=%v%%); dropped recommendation\n", targetPct, rec.Service, rec.Region, rec.ResourceType, rec.AverageInstancesUsedPerHour, rec.ExistingCoveragePct) return common.DropTargetInputInvalid } diff --git a/pkg/recfilter/sizing_test.go b/pkg/recfilter/sizing_test.go index b057d199..26ec55ce 100644 --- a/pkg/recfilter/sizing_test.go +++ b/pkg/recfilter/sizing_test.go @@ -2,6 +2,7 @@ package recfilter import ( "fmt" + "math" "math/big" "strings" "testing" @@ -451,3 +452,15 @@ func TestApplyTargetCoverage_UnknownCoverageNoAvg_AzureGCPPassThrough(t *testing assert.Equal(t, rec, out[0]) assert.True(t, d.IsEmpty()) } + +// The invalid-input drop message formats the target like the sibling drop +// messages (%.1f%%), not with %v (#227). +func TestApplyTargetCoverage_InvalidInputLogFormatsTarget(t *testing.T) { + t.Parallel() + rec := mkRI(10, math.NaN(), 0) + var logs []string + out := ApplyTargetCoverage([]common.Recommendation{rec}, 75, captureLogf(&logs), nil) + assert.Empty(t, out) + require.Len(t, logs, 1) + assert.Contains(t, logs[0], "--target-coverage=75.0% has invalid or out-of-range inputs") +} diff --git a/providers/aws/provider.go b/providers/aws/provider.go index 8c801a8f..0654d1f0 100644 --- a/providers/aws/provider.go +++ b/providers/aws/provider.go @@ -298,6 +298,7 @@ func (p *Provider) appendOrgAccounts(ctx context.Context, accounts []common.Acco output, err := paginator.NextPage(ctx) if err != nil { if page == 0 && isOrgListAccountsSilentError(err) { + logging.Infof("AWS Organizations ListAccounts unavailable, using the caller account only: %v", err) return accounts, nil } logging.Warnf("AWS Organizations ListAccounts pagination failed mid-run: %v", err) diff --git a/providers/aws/provider_test.go b/providers/aws/provider_test.go index 718f1584..5b6381c8 100644 --- a/providers/aws/provider_test.go +++ b/providers/aws/provider_test.go @@ -1,6 +1,7 @@ package aws import ( + "bytes" "context" "errors" "testing" @@ -17,6 +18,7 @@ import ( "github.com/stretchr/testify/require" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" ) @@ -495,10 +497,15 @@ func TestAWSProvider_GetAccounts_WithMock(t *testing.T) { errOnPage: 0, }) + var logs bytes.Buffer + prev := logging.SetOutput(&logs) + t.Cleanup(func() { logging.SetOutput(prev) }) + accounts, err := p.GetAccounts(context.Background()) require.NoError(t, err) require.Len(t, accounts, 1) assert.Equal(t, "123456789012", accounts[0].ID) + assert.Contains(t, logs.String(), "AWS Organizations ListAccounts unavailable") }) t.Run("returns current account when Organizations access is denied", func(t *testing.T) { @@ -516,10 +523,15 @@ func TestAWSProvider_GetAccounts_WithMock(t *testing.T) { errOnPage: 0, }) + var logs bytes.Buffer + prev := logging.SetOutput(&logs) + t.Cleanup(func() { logging.SetOutput(prev) }) + accounts, err := p.GetAccounts(context.Background()) require.NoError(t, err) require.Len(t, accounts, 1) assert.Equal(t, "123456789012", accounts[0].ID) + assert.Contains(t, logs.String(), "AWS Organizations ListAccounts unavailable") }) t.Run("propagates AccessDenied after a page succeeded instead of returning a partial org list", func(t *testing.T) { @@ -594,6 +606,65 @@ func TestAWSProvider_GetAccounts_WithMock(t *testing.T) { assert.Contains(t, err.Error(), "network blew up") }) + newOrgProvider := func(pg *mockOrganizationsPaginator) *Provider { + p := &Provider{cfg: aws.Config{Region: "us-east-1"}} + p.SetSTSClient(&mockSTSClient{ + getCallerIdentityFunc: func(ctx context.Context, params *sts.GetCallerIdentityInput, optFns ...func(*sts.Options)) (*sts.GetCallerIdentityOutput, error) { + return &sts.GetCallerIdentityOutput{Account: aws.String("111111111111")}, nil + }, + }) + p.SetOrganizationsPaginator(pg) + return p + } + orgPage := func(ids ...string) *organizations.ListAccountsOutput { + out := &organizations.ListAccountsOutput{} + for _, id := range ids { + out.Accounts = append(out.Accounts, orgtypes.Account{Id: aws.String(id), Name: aws.String("acct-" + id)}) + } + return out + } + + t.Run("accumulates accounts across two successful pages", func(t *testing.T) { + p := newOrgProvider(&mockOrganizationsPaginator{ + pages: []*organizations.ListAccountsOutput{orgPage("222222222222"), orgPage("333333333333")}, + errOnPage: -1, + }) + + accounts, err := p.GetAccounts(context.Background()) + require.NoError(t, err) + require.Len(t, accounts, 3) + assert.Equal(t, "333333333333", accounts[2].ID) + }) + + t.Run("accumulates accounts across three or more pages and skips the caller account", func(t *testing.T) { + p := newOrgProvider(&mockOrganizationsPaginator{ + pages: []*organizations.ListAccountsOutput{ + orgPage("222222222222"), orgPage("111111111111", "333333333333"), orgPage("444444444444"), orgPage("555555555555"), + }, + errOnPage: -1, + }) + + accounts, err := p.GetAccounts(context.Background()) + require.NoError(t, err) + ids := make([]string, 0, len(accounts)) + for _, a := range accounts { + ids = append(ids, a.ID) + } + assert.Equal(t, []string{"111111111111", "222222222222", "333333333333", "444444444444", "555555555555"}, ids) + }) + + t.Run("propagates NotInUse on page two instead of falling back", func(t *testing.T) { + p := newOrgProvider(&mockOrganizationsPaginator{ + pages: []*organizations.ListAccountsOutput{orgPage("222222222222"), {}}, + nextErr: &smithy.GenericAPIError{Code: "AWSOrganizationsNotInUseException", Message: "not in org"}, + errOnPage: 1, + }) + + _, err := p.GetAccounts(context.Background()) + require.Error(t, err) + assert.Contains(t, err.Error(), "AWSOrganizationsNotInUseException") + }) + t.Run("returns error when STS fails", func(t *testing.T) { p := &Provider{ cfg: aws.Config{Region: "us-east-1"},