Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions cmd/kosli/byteSize.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package main

import (
"errors"
"fmt"
"math"
"strconv"
"strings"
"unicode"
)

// byteSizeUnits maps a lower-cased unit, with any trailing "b" or "ib" already
// stripped, to bytes. Units are binary, as disk figures are.
var byteSizeUnits = map[string]int64{
"": 1 << 20, // a bare number is megabytes
"b": 1,
"k": 1 << 10,
"m": 1 << 20,
"g": 1 << 30,
"t": 1 << 40,
}

// parseByteSize turns "512", "512M", "8GB" or "1.5G" into bytes: a bare number
// is megabytes, a K, M, G or T suffix takes an optional B, and "B" alone is bytes.
func parseByteSize(s string) (int64, error) {
s = strings.TrimSpace(s)
if s == "" {
return 0, errors.New("size is empty")
}

digits := 0
for digits < len(s) && (s[digits] >= '0' && s[digits] <= '9' || s[digits] == '.') {
digits++
}
number, unit := s[:digits], strings.TrimSpace(s[digits:])
if number == "" || strings.ContainsFunc(unit, func(r rune) bool { return !unicode.IsLetter(r) }) {
return 0, fmt.Errorf("%q is not a size: expected a number with an optional K, M, G or T unit, e.g. 512M", s)
}
value, err := strconv.ParseFloat(number, 64)
if err != nil {
return 0, fmt.Errorf("%q is not a size: expected a number with an optional K, M, G or T unit, e.g. 512M", s)
}

key := strings.ToLower(unit)
if key != "" && key != "b" {
// Only a single unit letter may precede the optional "b" or "ib", so
// "ib" alone and "bb" are unknown rather than a guess at megabytes or bytes.
key = strings.TrimSuffix(strings.TrimSuffix(key, "ib"), "b")
if len(key) != 1 || key == "b" {
return 0, fmt.Errorf("unknown unit %q in size %q: use K, M, G or T, optionally followed by B", unit, s)
}
}
multiplier, ok := byteSizeUnits[key]
if !ok {
return 0, fmt.Errorf("unknown unit %q in size %q: use K, M, G or T, optionally followed by B", unit, s)
}
Comment thread
mbevc1 marked this conversation as resolved.

bytes := value * float64(multiplier)
if bytes >= math.MaxInt64 {
return 0, fmt.Errorf("size %q is too large", s)
}
if bytes < 1 {
return 0, fmt.Errorf("size %q must be at least 1 byte", s)
}
return int64(bytes), nil
}
68 changes: 68 additions & 0 deletions cmd/kosli/byteSize_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package main

import (
"testing"

"github.com/kosli-dev/cli/internal/aws"
"github.com/stretchr/testify/require"
)

func TestParseByteSize(t *testing.T) {
const mib = int64(1) << 20
for _, tc := range []struct {
input string
want int64
wantErr string
}{
{input: "512", want: 512 * mib},
{input: "1", want: mib},
{input: " 64 ", want: 64 * mib},
{input: "512M", want: 512 * mib},
{input: "512MB", want: 512 * mib},
{input: "512mb", want: 512 * mib},
{input: "512MiB", want: 512 * mib},
{input: "8G", want: 8 << 30},
{input: "8GB", want: 8 << 30},
{input: "8 GB", want: 8 << 30},
{input: "2T", want: 2 << 40},
{input: "1024K", want: 1 << 20},
{input: "4096KB", want: 4 << 20},
{input: "1000B", want: 1000},
{input: "1.5G", want: 3 << 29},
{input: "0.5M", want: 512 << 10},
{input: "", wantErr: "empty"},
{input: "0", wantErr: "must be at least 1 byte"},
{input: "0B", wantErr: "must be at least 1 byte"},
{input: "-1", wantErr: "not a size"},
{input: "-512M", wantErr: "not a size"},
{input: "abc", wantErr: "not a size"},
{input: "M", wantErr: "not a size"},
{input: "512X", wantErr: `unknown unit "X"`},
{input: "5ib", wantErr: `unknown unit "ib"`},
{input: "5bb", wantErr: `unknown unit "bb"`},
{input: "5KiBB", wantErr: `unknown unit "KiBB"`},
{input: "512 megabytes", wantErr: `unknown unit "megabytes"`},
{input: "1e3", wantErr: "not a size"},
{input: "0x10", wantErr: "not a size"},
{input: "1,024", wantErr: "not a size"},
{input: "99999999999T", wantErr: "too large"},
} {
t.Run(tc.input, func(t *testing.T) {
got, err := parseByteSize(tc.input)
if tc.wantErr != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tc.wantErr)
return
}
require.NoError(t, err)
require.Equal(t, tc.want, got)
})
}
}

// The flag default is a string, so it can drift from the value it spells.
func TestDefaultDownloadBudgetMatchesTheAwsDefault(t *testing.T) {
got, err := parseByteSize(defaultDownloadBudget)
require.NoError(t, err)
require.Equal(t, aws.DefaultDownloadLimits.BytesInFlight, got)
}
2 changes: 2 additions & 0 deletions cmd/kosli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,8 @@ Paths the list already matches stay excluded whatever is later added there, so k
awsSecretKeyFlag = "The AWS secret access key."
awsRegionFlag = "The AWS region."
bucketNameFlag = "The name of the S3 bucket."
downloadConcurrencyFlag = "[optional] The number of S3 objects to download at the same time when fingerprinting the bucket. Each object in flight may hold up to 40 MB of download buffers in memory, on top of the disk the --download-budget allows."
downloadBudgetFlag = "[optional] The maximum total size of the S3 objects downloading at the same time, which caps the temporary disk the snapshot uses. A bare number is megabytes; add K, M, G or T (optionally followed by B) to choose the unit, e.g. 512M or 8G. An object larger than the budget still downloads, on its own. Objects are downloaded to the OS temporary directory."
bucketPathsFlag = "[optional] The comma separated list of file and/or directory paths in the S3 bucket to include when fingerprinting. Paths match by literal prefix. Cannot be used together with --exclude or --exclude-regex."
bucketPathsRegexFlag = "[optional] The comma separated list of Go regular expressions matched against object keys in the S3 bucket to include when fingerprinting. Cannot be used together with --exclude or --exclude-regex."
excludeBucketPathsFlag = "[optional] The comma separated list of file and/or directory paths in the S3 bucket to exclude when fingerprinting. Paths match by literal prefix. Cannot be used together with --include or --include-regex."
Expand Down
38 changes: 30 additions & 8 deletions cmd/kosli/snapshotS3.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"fmt"
"io"
"net/http"
"net/url"
Expand Down Expand Up @@ -71,12 +72,15 @@ kosli snapshot s3 yourEnvironmentName \
`

type snapshotS3Options struct {
bucket string
includePaths []string
includeRegex []string
excludePaths []string
excludeRegex []string
awsStaticCreds *aws.AWSStaticCreds
bucket string
includePaths []string
includeRegex []string
excludePaths []string
excludeRegex []string
downloadConcurrency int
downloadBudget string
downloadLimits aws.DownloadLimits
awsStaticCreds *aws.AWSStaticCreds
}

func newSnapshotS3Cmd(out io.Writer) *cobra.Command {
Expand Down Expand Up @@ -108,7 +112,7 @@ func newSnapshotS3Cmd(out io.Writer) *cobra.Command {
}
}

return nil
return o.resolveDownloadLimits()
},
RunE: func(cmd *cobra.Command, args []string) error {
return o.run(args)
Expand All @@ -120,6 +124,8 @@ func newSnapshotS3Cmd(out io.Writer) *cobra.Command {
cmd.Flags().StringSliceVar(&o.includeRegex, "include-regex", []string{}, bucketPathsRegexFlag)
cmd.Flags().StringSliceVarP(&o.excludePaths, "exclude", "x", []string{}, excludeBucketPathsFlag)
cmd.Flags().StringSliceVar(&o.excludeRegex, "exclude-regex", []string{}, excludeBucketPathsRegexFlag)
cmd.Flags().IntVar(&o.downloadConcurrency, "download-concurrency", aws.DefaultDownloadLimits.Concurrency, downloadConcurrencyFlag)
cmd.Flags().StringVar(&o.downloadBudget, "download-budget", defaultDownloadBudget, downloadBudgetFlag)
addAWSAuthFlags(cmd, o.awsStaticCreds)
addDryRunFlag(cmd)

Expand All @@ -143,7 +149,7 @@ func (o *snapshotS3Options) run(args []string) error {
return err
}

s3Data, err := o.awsStaticCreds.GetS3Data(o.bucket, o.includePaths, o.includeRegex, o.excludePaths, o.excludeRegex, logger)
s3Data, err := o.awsStaticCreds.GetS3Data(o.bucket, o.includePaths, o.includeRegex, o.excludePaths, o.excludeRegex, o.downloadLimits, logger)
if err != nil {
return err
}
Expand All @@ -164,3 +170,19 @@ func (o *snapshotS3Options) run(args []string) error {
}
return err
}

// defaultDownloadBudget is aws.DefaultDownloadLimits.BytesInFlight as the flag
// spells it; a test keeps the two equal.
const defaultDownloadBudget = "512M"

func (o *snapshotS3Options) resolveDownloadLimits() error {
if o.downloadConcurrency < 1 {
return fmt.Errorf("--download-concurrency must be at least 1, got %d", o.downloadConcurrency)
}
budget, err := parseByteSize(o.downloadBudget)
if err != nil {
return fmt.Errorf("invalid --download-budget: %w", err)
}
o.downloadLimits = aws.DownloadLimits{Concurrency: o.downloadConcurrency, BytesInFlight: budget}
return nil
}
28 changes: 28 additions & 0 deletions cmd/kosli/snapshotS3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,34 @@ func (suite *SnapshotS3TestSuite) TestSnapshotS3Cmd() {
cmd: fmt.Sprintf(`snapshot s3 %s %s --bucket %s --exclude dummy`, suite.envName, suite.defaultKosliArguments, suite.bucketName),
golden: "bucket kosli-cli-public was reported to environment snapshot-s3-env\n",
},
{
name: "download limits can be set, with a bare number read as megabytes",
cmd: fmt.Sprintf(`snapshot s3 %s %s --bucket %s --download-concurrency 2 --download-budget 64`, suite.envName, suite.defaultKosliArguments, suite.bucketName),
golden: "bucket kosli-cli-public was reported to environment snapshot-s3-env\n",
},
{
name: "the download budget takes a unit suffix",
cmd: fmt.Sprintf(`snapshot s3 %s %s --bucket %s --download-budget 2GB`, suite.envName, suite.defaultKosliArguments, suite.bucketName),
golden: "bucket kosli-cli-public was reported to environment snapshot-s3-env\n",
},
{
wantError: true,
name: "snapshot s3 fails if --download-concurrency is below 1",
cmd: fmt.Sprintf(`snapshot s3 %s %s --bucket %s --download-concurrency 0`, suite.envName, suite.defaultKosliArguments, suite.bucketName),
golden: "Error: --download-concurrency must be at least 1, got 0\n",
},
{
wantError: true,
name: "snapshot s3 fails if --download-budget is not a size",
cmd: fmt.Sprintf(`snapshot s3 %s %s --bucket %s --download-budget large`, suite.envName, suite.defaultKosliArguments, suite.bucketName),
golden: "Error: invalid --download-budget: \"large\" is not a size: expected a number with an optional K, M, G or T unit, e.g. 512M\n",
},
{
wantError: true,
name: "snapshot s3 fails if --download-budget is zero",
cmd: fmt.Sprintf(`snapshot s3 %s %s --bucket %s --download-budget 0`, suite.envName, suite.defaultKosliArguments, suite.bucketName),
golden: "Error: invalid --download-budget: size \"0\" must be at least 1 byte\n",
},
}

for _, t := range tests {
Expand Down
2 changes: 2 additions & 0 deletions cmd/kosli/testdata/empty-flag-audit-coverage.json
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,8 @@
"aws-region": "string",
"aws-secret-key": "string",
"bucket": "string",
"download-budget": "string",
"download-concurrency": "int",
"dry-run": "bool",
"exclude": "stringSlice",
"exclude-regex": "stringSlice",
Expand Down
8 changes: 4 additions & 4 deletions docs/adr/20260911-s3-fingerprint-from-virtual-tree.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
title: "20260911 - Fingerprint S3 buckets from a virtual tree; object keys never become local paths"
description: "Download each object to an anonymous temp file, hash it, delete it, and compute the directory fingerprint from (key, sha256) pairs so that no S3 key is ever used as a filename"
status: "Proposed"
status: "Accepted"
date: "2026-09-11"
---

Expand Down Expand Up @@ -37,7 +37,7 @@ The fingerprint format itself is fixed. An S3 snapshot must match the fingerprin

4. **A root `.kosli_ignore` is honoured virtually.** Its rules are parsed by `digest.ParseIgnoreRules`, the same reading `DirSha256` gives the file, and resolved by `digest/virtualglob.go`, which reproduces `filepathx.Glob`, `filepath.Glob` and `filepath.Walk` step for step over the virtual tree rather than reimplementing what the globs appear to mean. That is what keeps their quirks identical: a literal `**/x` finds a root `x` spelled with a double slash, which the walk's cleaned paths never equal, so a root file `x` survives while a root directory `x` keeps its name and loses its contents; `**/*.log` is rebuilt cleaned and matches outright; and excluding `logs/*` leaves an empty directory whose name is still hashed. Exclusion therefore runs inside the tree walk, not by filtering the file list. The ignore file can never exclude itself, as in `DirSha256`. `digest.FilesNeedingContent` shares that walk so excluded objects are not downloaded at all, and `VirtualDirSha256` refuses a tree that needs a digest it was not given, so a skipped download can never leak into a fingerprint. Equivalence with `DirSha256` on a materialised tree is asserted for every rule shape in `TestVirtualIgnoreTestSuite`.

5. **Downloads may run in parallel**, behind a fixed worker pool and a bytes-in-flight budget derived from listing sizes, with results written by index for determinism and a cancellable context so the first transport error stops in-flight multipart downloads. This is a performance property, not a safety one, and is delivered separately from the change this record describes; see #1167. Until it lands, downloads are sequential, exactly as before, and peak temp disk is one object.
5. **Downloads may run in parallel**, behind a fixed worker pool and a bytes-in-flight budget derived from listing sizes, with results written by index for determinism and a cancellable context so the first transport error stops in-flight multipart downloads. This is a performance property, not a safety one, and was delivered separately from the change this record describes; see #1167. The defaults are eight objects in flight within 512 MB of listed bytes, tunable with `--download-concurrency` and `--download-budget`, so peak temp disk is the budget rather than one object.

6. **The switch is pinned, not argued.** `TestPinnedFingerprints` in `internal/aws` holds fingerprints of representative fake buckets recorded against the key-layout implementation before it was replaced: unusual key shapes, a `.` sorting before a `/`, nested prefixes with folder markers, and a single object with its basename as artifact name. It was green before the switch and must stay green after it, alongside `TestGetS3DataFromClientKeepsTodaysLayoutForUnusualKeys` from #1155.

Expand All @@ -62,6 +62,6 @@ The attest side is unchanged: `kosli attest artifact --artifact-type dir` still
- `localPathForS3Key`, `filepath.IsLocal`, the `O_EXCL` open, the `ENOTDIR` branch, `containsSingleFile` and the platform-conditional tests from #1155 are deleted. No code in the S3 path branches on the operating system any more. The codebase does not get smaller, though: the virtual tree, the key rule with its collision reporting, and above all the faithful simulation of `filepathx.Glob` add several hundred lines, most of them owed to reproducing `.kosli_ignore` semantics exactly. That is the price of the compatibility contract, paid once and shared with #1069.
- The bucket's ignore file is recognised by the exact key `.kosli_ignore`. On disk, `ignoreFilePathInTree` also accepted a case-folded spelling such as `.KOSLI_IGNORE` where the operator's filesystem folded case, so on macOS or Windows such a bucket had its rules applied; now it does not, its exclusions stop applying and its fingerprint moves to the Linux value. Same correction as the key case above, and release-noted with it.
- `...`, `.. `, `CON`, colon and backslash keys snapshot again. `..` segments and colliding keys remain errors and now name every key involved.
- Excluded and ignored objects are not downloaded. Each object's temp file is removed once hashed, so peak temp disk falls from the whole bucket to one object, and to the configured budget once #1167 lands. The other half of that trade is memory: the old loop streamed the listing a page at a time, while a tree cannot be fingerprinted without holding it, so peak memory rises from one listing page to every object that survives the filters, at a few hundred bytes per object on top of its key across the listing, the key-to-path maps, the manifest and the tree. A million objects is a few hundred megabytes. #1167 sizes its budget against this figure.
- Excluded and ignored objects are not downloaded. Each object's temp file is removed once hashed, so peak temp disk falls from the whole bucket to one object, and to the configured budget now that #1167 has landed. The other half of that trade is memory: the old loop streamed the listing a page at a time, while a tree cannot be fingerprinted without holding it, so peak memory rises from one listing page to every object that survives the filters, at a few hundred bytes per object on top of its key across the listing, the key-to-path maps, the manifest and the tree. A million objects is a few hundred megabytes. #1167 sized its budget against this figure.
- #1069 rebases onto the shared layer: metadata mode becomes a sha256 source plugged into the same list, normalise, exclude, tree pipeline, its key rule disappears in favour of rule 2, and its rejection of buckets with a root `.kosli_ignore` becomes a download of that one object.
- Delivery is two pull requests. The first carries this decision with sequential downloads, so the security-relevant review is not mixed with performance work. The second, #1167, adds parallel downloads, the byte budget and the flags that tune them.
- Delivery was two pull requests. The first, #1180, carried this decision with sequential downloads, so the security-relevant review was not mixed with performance work. The second, #1167, added parallel downloads, the byte budget and the flags that tune them.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ require (
github.com/zalando/go-keyring v0.2.8
gitlab.com/gitlab-org/api/client-go v1.46.0
golang.org/x/oauth2 v0.37.0
golang.org/x/sync v0.22.0
golang.org/x/term v0.46.0
google.golang.org/api v0.297.0
google.golang.org/grpc v1.83.2
Expand Down Expand Up @@ -249,7 +250,6 @@ require (
golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f // indirect
golang.org/x/mod v0.38.0 // indirect
golang.org/x/net v0.58.0 // indirect
golang.org/x/sync v0.22.0 // indirect
golang.org/x/sys v0.48.0 // indirect
golang.org/x/text v0.41.0 // indirect
golang.org/x/time v0.15.0 // indirect
Expand Down
Loading
Loading