diff --git a/cmd/cli/app/artifact/artifact_list.go b/cmd/cli/app/artifact/artifact_list.go index 32b3b31d4d..1584231e80 100644 --- a/cmd/cli/app/artifact/artifact_list.go +++ b/cmd/cli/app/artifact/artifact_list.go @@ -4,14 +4,12 @@ package artifact import ( - "context" "fmt" "strings" "time" "github.com/spf13/cobra" "github.com/spf13/viper" - "google.golang.org/grpc" "github.com/mindersec/minder/cmd/cli/app" "github.com/mindersec/minder/internal/util" @@ -25,12 +23,22 @@ var listCmd = &cobra.Command{ Use: "list", Short: "List artifacts from a provider", Long: `The artifact list subcommand will list artifacts from a provider.`, - RunE: cli.GRPCClientWrapRunE(listCommand), + RunE: listCommand, } // listCommand is the artifact list subcommand -func listCommand(ctx context.Context, cmd *cobra.Command, _ []string, conn *grpc.ClientConn) error { - client := minderv1.NewArtifactServiceClient(conn) +func listCommand(cmd *cobra.Command, _ []string) error { + if err := viper.BindPFlags(cmd.Flags()); err != nil { + return fmt.Errorf("error binding flags: %w", err) + } + + client, cleanup, err := cli.GetCLIClient(cmd, minderv1.NewArtifactServiceClient) + if err != nil { + return err + } + defer cleanup() + + ctx := cmd.Context() provider := viper.GetString("provider") project := viper.GetString("project") @@ -74,17 +82,24 @@ func listCommand(ctx context.Context, cmd *cobra.Command, _ []string, conn *grpc } t.Render() case app.JSON: - out, err := util.GetJsonFromProto(artifactList) - if err != nil { - return cli.MessageAndError("Error getting json from proto", err) + // Emit each artifact as a separate JSON object (JSON stream) + for _, art := range artifactList.Results { + out, err := util.GetJsonFromProto(art) + if err != nil { + return cli.MessageAndError("Error getting json from proto", err) + } + if _, err := cmd.OutOrStdout().Write([]byte(out + "\n")); err != nil { + return cli.MessageAndError("Error writing json to stdout", err) + } } - cmd.Println(out) case app.YAML: out, err := util.GetYamlFromProto(artifactList) if err != nil { return cli.MessageAndError("Error getting yaml from proto", err) } - cmd.Println(out) + if _, err := cmd.OutOrStdout().Write([]byte(out + "\n")); err != nil { + return cli.MessageAndError("Error writing yaml to stdout", err) + } } return nil diff --git a/cmd/cli/app/artifact/artifact_list_test.go b/cmd/cli/app/artifact/artifact_list_test.go new file mode 100644 index 0000000000..f260b60e9b --- /dev/null +++ b/cmd/cli/app/artifact/artifact_list_test.go @@ -0,0 +1,46 @@ +// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package artifact + +import ( + "context" + "testing" + + "go.uber.org/mock/gomock" + + "github.com/mindersec/minder/internal/util/cli" + minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" + mockv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1/mock" +) + +//nolint:paralleltest // Uses global viper/Stdout state +func TestArtifactListCommand(t *testing.T) { + setupSuccess := func(t *testing.T, ctrl *gomock.Controller) context.Context { + t.Helper() + + artifactClient := mockv1.NewMockArtifactServiceClient(ctrl) + + listResp := &minderv1.ListArtifactsResponse{} + cli.LoadFixture(t, "mock_artifact_list.json", listResp) + + artifactClient.EXPECT(). + ListArtifacts(gomock.Any(), gomock.Any()). + Return(listResp, nil). + Times(1) + + ctx := cli.WithCLIClient[minderv1.ArtifactServiceClient](context.Background(), artifactClient) + return ctx + } + + tests := []cli.CmdTestCase{ + { + Name: "list artifacts - json output", + Args: []string{"artifact", "list", "-o", "json"}, + MockSetup: setupSuccess, + GoldenFileName: "artifact_list.json", + }, + } + + cli.RunCmdTests(t, tests, ArtifactCmd) +} diff --git a/cmd/cli/app/artifact/fixture/mock_artifact_list.json b/cmd/cli/app/artifact/fixture/mock_artifact_list.json new file mode 100644 index 0000000000..dea2efe253 --- /dev/null +++ b/cmd/cli/app/artifact/fixture/mock_artifact_list.json @@ -0,0 +1,22 @@ +{ + "results": [ + { + "artifactPk": "1", + "name": "artifact-1", + "type": "image", + "owner": "owner-1", + "repository": "org/repo", + "visibility": "public", + "createdAt": "2024-01-02T15:04:05Z" + }, + { + "artifactPk": "2", + "name": "artifact-2", + "type": "image", + "owner": "owner-2", + "repository": "org/repo2", + "visibility": "private", + "createdAt": "2024-02-02T15:04:05Z" + } + ] +} diff --git a/cmd/cli/app/artifact/testdata/artifact_list.json.golden b/cmd/cli/app/artifact/testdata/artifact_list.json.golden new file mode 100644 index 0000000000..bc34e1c1c8 --- /dev/null +++ b/cmd/cli/app/artifact/testdata/artifact_list.json.golden @@ -0,0 +1,2 @@ +{"artifactPk":"1","name":"artifact-1","type":"image","owner":"owner-1","repository":"org/repo","visibility":"public","createdAt":"2024-01-02T15:04:05Z"} +{"artifactPk":"2","name":"artifact-2","type":"image","owner":"owner-2","repository":"org/repo2","visibility":"private","createdAt":"2024-02-02T15:04:05Z"} diff --git a/internal/util/cli/context.go b/internal/util/cli/context.go index 7a04fd7dca..ab412b998c 100644 --- a/internal/util/cli/context.go +++ b/internal/util/cli/context.go @@ -22,6 +22,12 @@ func WithRPCClient[T any](ctx context.Context, client T) context.Context { return context.WithValue(ctx, key, client) } +// WithCLIClient is an alias for WithRPCClient kept for backwards/alternate usage +// by tests or callers expecting a "CLI"-named helper. +func WithCLIClient[T any](ctx context.Context, client T) context.Context { + return WithRPCClient[T](ctx, client) +} + // GetRPCClient extracts the generic RPC client from the provided context. func GetRPCClient[T any](ctx context.Context) (T, bool) { key := rpcKey{clientType: reflect.TypeOf((*T)(nil)).Elem()} diff --git a/internal/util/cli/testing.go b/internal/util/cli/testing.go index fd47a6b344..8552fe9b06 100644 --- a/internal/util/cli/testing.go +++ b/internal/util/cli/testing.go @@ -8,8 +8,10 @@ import ( "context" "encoding/json" "flag" + "io" "os" "path/filepath" + "strings" "testing" "github.com/spf13/cobra" @@ -122,13 +124,54 @@ func checkGoldenFile(t *testing.T, filename string, actual string) { expected, err := os.ReadFile(goldenPath) require.NoError(t, err, "could not read golden file. Run 'go test ./... -update' to generate it") + // Try standard JSON comparison first if json.Valid(expected) && json.Valid([]byte(actual)) { - // if it's valid json compare the objects (ignores spaces/newlines) require.JSONEq(t, string(expected), actual, "JSON Output does not match golden file") - } else { - // if it's a table, txt, or yaml fallback to exact string matching - require.Equal(t, string(expected), actual, "Output does not match golden file") + return } + + // Try to handle JSON streams (newline-delimited JSON) + // Attempt to decode newline-delimited JSON (JSON stream). + // If decoding fails, input is treated as non-JSON output. + expectedParsed, expectedErr := tryParseJSONStream(string(expected)) + actualParsed, actualErr := tryParseJSONStream(actual) + + // If both successfully parsed as JSON streams, compare them. + // This includes edge cases like empty arrays [] which are valid JSON. + if expectedErr == nil && actualErr == nil { + require.Equal(t, expectedParsed, actualParsed, "JSON stream output does not match golden file") + return + } + + // Otherwise do exact string matching (for tables, txt, yaml, etc.) + require.Equal(t, string(expected), actual, "Output does not match golden file") +} + +// tryParseJSONStream attempts to parse newline-delimited JSON (JSON stream output from CLI). +// Returns empty slice and an error if input is not a JSON stream (e.g., YAML, table, etc). +func tryParseJSONStream(input string) ([]interface{}, error) { + // Try to decode as JSON stream directly without heuristics. + // If the input is not JSON, the decoder will fail on the first non-JSON line. + dec := json.NewDecoder(strings.NewReader(input)) + + var result []interface{} + + for { + var obj interface{} + err := dec.Decode(&obj) + + if err == io.EOF { + break + } + if err != nil { + // Not a JSON stream (could be YAML, table, plain text, etc.) + return nil, err + } + + result = append(result, obj) + } + + return result, nil } // LoadFixture reads a JSON file from the "fixture" directory and unmarshals diff --git a/internal/util/cli/testing_json_test.go b/internal/util/cli/testing_json_test.go new file mode 100644 index 0000000000..619c1319ae --- /dev/null +++ b/internal/util/cli/testing_json_test.go @@ -0,0 +1,73 @@ +// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestTryParseJSONStream_Stream tests parsing of newline-delimited JSON (the main use case). +func TestTryParseJSONStream_Stream(t *testing.T) { + t.Parallel() + input := `{"id":1} +{"id":2}` + + result, err := tryParseJSONStream(input) + require.NoError(t, err) + require.Len(t, result, 2) + + // Verify actual content was parsed correctly + obj1 := result[0].(map[string]interface{}) + require.Equal(t, float64(1), obj1["id"]) + + obj2 := result[1].(map[string]interface{}) + require.Equal(t, float64(2), obj2["id"]) +} + +// TestTryParseJSONStream_SingleObject tests parsing of a single JSON object (backward compatibility). +func TestTryParseJSONStream_SingleObject(t *testing.T) { + t.Parallel() + input := `{"id":1}` + + result, err := tryParseJSONStream(input) + require.NoError(t, err) + require.Len(t, result, 1) + + // Verify actual content was parsed correctly + obj := result[0].(map[string]interface{}) + require.Equal(t, float64(1), obj["id"]) +} + +// TestTryParseJSONStream_NonJSON tests that non-JSON input (like YAML) is rejected. +func TestTryParseJSONStream_NonJSON(t *testing.T) { + t.Parallel() + input := ` +name: sachin +age: 22 +` + + _, err := tryParseJSONStream(input) + require.Error(t, err) +} + +// TestTryParseJSONStream_EmptyArray tests the edge case of empty JSON array. +func TestTryParseJSONStream_EmptyArray(t *testing.T) { + t.Parallel() + input := `[]` + + result, err := tryParseJSONStream(input) + require.NoError(t, err) + require.Len(t, result, 1) // decoder treats [] as one decoded value +} + +// TestTryParseJSONStream_InvalidJSON tests that malformed JSON is properly rejected. +func TestTryParseJSONStream_InvalidJSON(t *testing.T) { + t.Parallel() + input := `{"id":1` + + _, err := tryParseJSONStream(input) + require.Error(t, err) +}