Use Windows compatible file name for downloaded attestations when running `gh attestation download` by malancas · Pull Request #10051 · cli/cli · GitHub
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
6 changes: 3 additions & 3 deletions .github/workflows/go.yml
5 changes: 5 additions & 0 deletions pkg/cmd/attestation/download/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ func NewDownloadCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Comman
Any associated bundle(s) will be written to a file in the
current directory named after the artifact's digest. For example, if the
digest is "sha256:1234", the file will be named "sha256:1234.jsonl".

Colons are special characters on Windows and cannot be used in
file names. To accommodate, a dash will be used to separate the algorithm
from the digest in the attestations file name. For example, if the digest
is "sha256:1234", the file will be named "sha256-1234.jsonl".
`, "`"),
Example: heredoc.Doc(`
# Download attestations for a local artifact linked with an organization
Expand Down
27 changes: 21 additions & 6 deletions pkg/cmd/attestation/download/download_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"fmt"
"net/http"
"runtime"
"strings"
"testing"

Expand All @@ -22,6 +23,17 @@ import (

var artifactPath = test.NormalizeRelativePath("../test/data/sigstore-js-2.1.0.tgz")

func expectedFilePath(tempDir string, digestWithAlg string) string {
var filename string
if runtime.GOOS == "windows" {
filename = fmt.Sprintf("%s.jsonl", strings.ReplaceAll(digestWithAlg, ":", "-"))
} else {
filename = fmt.Sprintf("%s.jsonl", digestWithAlg)
}

return test.NormalizeRelativePath(fmt.Sprintf("%s/%s", tempDir, filename))
}

func TestNewDownloadCmd(t *testing.T) {
testIO, _, _, _ := iostreams.Test()
f := &cmdutil.Factory{
Expand Down Expand Up @@ -201,9 +213,10 @@ func TestRunDownload(t *testing.T) {
artifact, err := artifact.NewDigestedArtifact(baseOpts.OCIClient, baseOpts.ArtifactPath, baseOpts.DigestAlgorithm)
require.NoError(t, err)

require.FileExists(t, fmt.Sprintf("%s/%s.jsonl", tempDir, artifact.DigestWithAlg()))
expectedFilePath := expectedFilePath(tempDir, artifact.DigestWithAlg())
require.FileExists(t, expectedFilePath)

actualLineCount, err := countLines(fmt.Sprintf("%s/%s.jsonl", tempDir, artifact.DigestWithAlg()))
actualLineCount, err := countLines(expectedFilePath)
require.NoError(t, err)

expectedLineCount := 2
Expand All @@ -221,9 +234,10 @@ func TestRunDownload(t *testing.T) {
artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm)
require.NoError(t, err)

require.FileExists(t, fmt.Sprintf("%s/%s.jsonl", tempDir, artifact.DigestWithAlg()))
expectedFilePath := expectedFilePath(tempDir, artifact.DigestWithAlg())
require.FileExists(t, expectedFilePath)

actualLineCount, err := countLines(fmt.Sprintf("%s/%s.jsonl", tempDir, artifact.DigestWithAlg()))
actualLineCount, err := countLines(expectedFilePath)
require.NoError(t, err)

expectedLineCount := 2
Expand All @@ -240,9 +254,10 @@ func TestRunDownload(t *testing.T) {
artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm)
require.NoError(t, err)

require.FileExists(t, fmt.Sprintf("%s/%s.jsonl", tempDir, artifact.DigestWithAlg()))
expectedFilePath := expectedFilePath(tempDir, artifact.DigestWithAlg())
require.FileExists(t, expectedFilePath)

actualLineCount, err := countLines(fmt.Sprintf("%s/%s.jsonl", tempDir, artifact.DigestWithAlg()))
actualLineCount, err := countLines(expectedFilePath)
require.NoError(t, err)

expectedLineCount := 2
Expand Down
8 changes: 8 additions & 0 deletions pkg/cmd/attestation/download/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"errors"
"fmt"
"os"
"runtime"
"strings"

"github.com/cli/cli/v2/pkg/cmd/attestation/api"
)
Expand All @@ -20,6 +22,12 @@ type LiveStore struct {
}

func (s *LiveStore) createJSONLinesFilePath(artifact string) string {
if runtime.GOOS == "windows" {
// Colons are special characters in Windows and cannot be used in file names.
// Replace them with dashes to avoid issues.
artifact = strings.ReplaceAll(artifact, ":", "-")
}

path := fmt.Sprintf("%s.jsonl", artifact)
if s.outputPath != "" {
return fmt.Sprintf("%s/%s", s.outputPath, path)
Expand Down
16 changes: 11 additions & 5 deletions pkg/cmd/attestation/download/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"os"
"path"
"runtime"
"testing"

"github.com/cli/cli/v2/pkg/cmd/attestation/api"
Expand All @@ -31,7 +32,12 @@ func TestCreateJSONLinesFilePath(t *testing.T) {
artifact, err := artifact.NewDigestedArtifact(oci.MockClient{}, "../test/data/sigstore-js-2.1.0.tgz", "sha512")
require.NoError(t, err)

outputFileName := fmt.Sprintf("%s.jsonl", artifact.DigestWithAlg())
var expectedFileName string
if runtime.GOOS == "windows" {
expectedFileName = fmt.Sprintf("%s-%s.jsonl", artifact.Algorithm(), artifact.Digest())
} else {
expectedFileName = fmt.Sprintf("%s.jsonl", artifact.DigestWithAlg())
}

testCases := []struct {
name string
Expand All @@ -41,22 +47,22 @@ func TestCreateJSONLinesFilePath(t *testing.T) {
{
name: "with output path",
outputPath: tempDir,
expected: path.Join(tempDir, outputFileName),
expected: path.Join(tempDir, expectedFileName),
},
{
name: "with nested output path",
outputPath: path.Join(tempDir, "subdir"),
expected: path.Join(tempDir, "subdir", outputFileName),
expected: path.Join(tempDir, "subdir", expectedFileName),
},
{
name: "with output path with beginning slash",
outputPath: path.Join("/", tempDir, "subdir"),
expected: path.Join("/", tempDir, "subdir", outputFileName),
expected: path.Join("/", tempDir, "subdir", expectedFileName),
},
{
name: "without output path",
outputPath: "",
expected: outputFileName,
expected: expectedFileName,
},
}

Expand Down
Binary file not shown.
47 changes: 47 additions & 0 deletions test/integration/attestation-cmd/download/download.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/usr/bin/env bash
set -euo pipefail

if [ "$#" -ne 1 ]; then
echo "Usage: $0 <matrix-os>"
exit 1
fi

os=$1

# Get the root directory of the repository
rootDir="$(git rev-parse --show-toplevel)"

ghBuildPath="$rootDir/bin/gh"

artifactPath="$rootDir/pkg/cmd/attestation/test/data/gh_2.60.1_windows_arm64.zip"

# Download attestations for the package
if ! $ghBuildPath attestation download "$artifactPath" --owner=cli; then
# cleanup test data
echo "Failed to download attestations"
exit 1
fi

digest="5ddb1d4d013a44c2e5df027867c0d4161383eb7c16e569a86384af52bfe09a65"
attestation_filename="sha256:$digest.jsonl"
if [ "$os" == "windows-latest" ]; then
echo "Running the test on Windows."
echo "Build the expected filename accordingly"
attestation_filename="sha256-$digest.jsonl"
fi

if [ ! -f "$attestation_filename" ]; then
echo "Expected attestation file $attestation_filename not found"
exit 1
fi

if [ ! -s "$attestation_filename" ]; then
echo "Attestation file $attestation_filename is empty"
rm "$attestation_filename"
exit 1
fi

cat "$attestation_filename"

# Clean up the downloaded attestation file
rm "$attestation_filename"
30 changes: 30 additions & 0 deletions test/integration/attestation-cmd/run-all-tests.sh