diff --git a/cmd/cli/app/artifact/artifact_get.go b/cmd/cli/app/artifact/artifact_get.go index f628b21519..53f7596ce0 100644 --- a/cmd/cli/app/artifact/artifact_get.go +++ b/cmd/cli/app/artifact/artifact_get.go @@ -14,10 +14,10 @@ import ( "github.com/spf13/cobra" "github.com/spf13/viper" - "google.golang.org/grpc" "google.golang.org/protobuf/reflect/protoreflect" "github.com/mindersec/minder/cmd/cli/app" + "github.com/mindersec/minder/cmd/cli/app/profile" "github.com/mindersec/minder/internal/util" "github.com/mindersec/minder/internal/util/cli" "github.com/mindersec/minder/internal/util/cli/table" @@ -30,12 +30,28 @@ var getCmd = &cobra.Command{ Use: "get", Short: "Get artifact details", Long: `The artifact get subcommand will get artifact details from an artifact, for a given ID.`, - RunE: cli.GRPCClientWrapRunE(getCommand), + RunE: getCommand, } // getCommand is the artifact get subcommand -func getCommand(ctx context.Context, cmd *cobra.Command, _ []string, conn *grpc.ClientConn) error { - client := minderv1.NewArtifactServiceClient(conn) +func getCommand(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() + + profileClient, profileCleanup, err := cli.GetCLIClient(cmd, minderv1.NewProfileServiceClient) + if err != nil { + return err + } + defer profileCleanup() + + ctx := cmd.Context() provider := viper.GetString("provider") project := viper.GetString("project") @@ -61,7 +77,7 @@ func getCommand(ctx context.Context, cmd *cobra.Command, _ []string, conn *grpc. return cli.MessageAndError("Error printing artifact", err) } - evalStatus, err := artifactEvalStatus(ctx, conn, art, provider, project) + evalStatus, err := artifactEvalStatus(ctx, profileClient, art, provider, project) if err != nil { return cli.MessageAndError("Error getting artifact evaluation status", err) } @@ -113,12 +129,12 @@ func artifactGet( } func artifactEvalStatus( - ctx context.Context, conn *grpc.ClientConn, + ctx context.Context, + client minderv1.ProfileServiceClient, artifact *minderv1.Artifact, provider, project string, ) ([]*minderv1.RuleEvaluationStatus, error) { - profClient := minderv1.NewProfileServiceClient(conn) - profiles, err := profClient.ListProfiles(ctx, &minderv1.ListProfilesRequest{ + profiles, err := client.ListProfiles(ctx, &minderv1.ListProfilesRequest{ Context: &minderv1.Context{ Provider: &provider, Project: &project, @@ -130,20 +146,20 @@ func artifactEvalStatus( var respList []*minderv1.RuleEvaluationStatus - for _, profile := range profiles.Profiles { + for _, prof := range profiles.Profiles { req := &minderv1.GetProfileStatusByNameRequest{ Context: &minderv1.Context{ Provider: &provider, Project: &project, }, - Name: profile.GetName(), + Name: prof.GetName(), Entity: &minderv1.EntityTypedId{ Id: artifact.ArtifactPk, Type: minderv1.Entity_ENTITY_ARTIFACTS, }, } - resp, err := profClient.GetProfileStatusByName(ctx, req) + resp, err := client.GetProfileStatusByName(ctx, req) if err != nil { return nil, cli.MessageAndError("Error getting profile status", err) } @@ -190,16 +206,23 @@ func printArtifact( func printEvalStatus( cmd *cobra.Command, evalStatus []*minderv1.RuleEvaluationStatus, format string, ) error { + if len(evalStatus) == 0 { + return nil + } + switch format { case app.Table: ta := table.New(table.Simple, layouts.Default, cmd.OutOrStdout(), - []string{"Profile", "Rule", "Status", "Message"}) + []string{"Profile", "Rule", "Result", "Details"}) for _, status := range evalStatus { + ruleName := profile.RuleDisplayName(status) + reasoning := profile.FormatEvaluationReasoning(status) + ta.AddRow( status.ProfileId, - status.RuleTypeName, + ruleName, status.Status, - status.Details, + reasoning, ) } ta.Render() diff --git a/cmd/cli/app/artifact/artifact_get_test.go b/cmd/cli/app/artifact/artifact_get_test.go new file mode 100644 index 0000000000..5ff94a5dcf --- /dev/null +++ b/cmd/cli/app/artifact/artifact_get_test.go @@ -0,0 +1,93 @@ +// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package artifact + +// JSON output is not tested because protojson formatting is not stable across environments. +// This can cause flaky tests due to spacing differences. +// See maintainer discussion in PR #6417. + +import ( + "context" + "testing" + + "go.uber.org/mock/gomock" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "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 // Cannot run in parallel because it swaps global Viper/Stdout state +func TestArtifactGetCommand(t *testing.T) { + setupSuccess := func(t *testing.T, ctrl *gomock.Controller) context.Context { + t.Helper() + + artifactClient := mockv1.NewMockArtifactServiceClient(ctrl) + profileClient := mockv1.NewMockProfileServiceClient(ctrl) + + artifactResp := &minderv1.GetArtifactByIdResponse{} + cli.LoadFixture(t, "mock_artifact_get.json", artifactResp) + + artifactClient.EXPECT(). + GetArtifactById(gomock.Any(), gomock.Any()). + Return(artifactResp, nil). + Times(1) + + listProfilesResp := &minderv1.ListProfilesResponse{} + cli.LoadFixture(t, "mock_list_profiles.json", listProfilesResp) + profileClient.EXPECT(). + ListProfiles(gomock.Any(), gomock.Any()). + Return(listProfilesResp, nil). + Times(1) + + statusResp := &minderv1.GetProfileStatusByNameResponse{} + cli.LoadFixture(t, "mock_profile_status.json", statusResp) + profileClient.EXPECT(). + GetProfileStatusByName(gomock.Any(), gomock.Any()). + Return(statusResp, nil). + Times(1) + + ctx := cli.WithRPCClient[minderv1.ArtifactServiceClient](context.Background(), artifactClient) + ctx = cli.WithRPCClient[minderv1.ProfileServiceClient](ctx, profileClient) + return ctx + } + + tests := []cli.CmdTestCase{ + { + Name: "get artifact - table output", + Args: []string{"artifact", "get", "-i", "111", "-o", "table"}, + MockSetup: setupSuccess, + GoldenFileName: "artifact_get.table", + }, + { + Name: "get artifact - yaml output", + Args: []string{"artifact", "get", "-i", "111", "-o", "yaml"}, + MockSetup: setupSuccess, + GoldenFileName: "artifact_get.yaml", + }, + { + Name: "server error handling", + Args: []string{"artifact", "get", "-i", "111"}, + MockSetup: func(t *testing.T, ctrl *gomock.Controller) context.Context { + t.Helper() + artifactClient := mockv1.NewMockArtifactServiceClient(ctrl) + profileClient := mockv1.NewMockProfileServiceClient(ctrl) + + artifactClient.EXPECT(). + GetArtifactById(gomock.Any(), gomock.Any()). + Return(nil, status.Error(codes.NotFound, "artifact not found")). + Times(1) + + ctx := cli.WithRPCClient[minderv1.ArtifactServiceClient](context.Background(), artifactClient) + ctx = cli.WithRPCClient[minderv1.ProfileServiceClient](ctx, profileClient) + return ctx + }, + ExpectedError: "artifact not found", + }, + } + + cli.RunCmdTests(t, tests, ArtifactCmd) +} diff --git a/cmd/cli/app/artifact/fixture/mock_artifact_get.json b/cmd/cli/app/artifact/fixture/mock_artifact_get.json new file mode 100644 index 0000000000..af4d672350 --- /dev/null +++ b/cmd/cli/app/artifact/fixture/mock_artifact_get.json @@ -0,0 +1,11 @@ +{ + "artifact": { + "artifactPk": "111", + "name": "artifact-1", + "type": "image", + "owner": "owner-1", + "repository": "org/repo", + "visibility": "public", + "createdAt": "2024-01-02T15:04:05Z" + } +} diff --git a/cmd/cli/app/artifact/fixture/mock_list_profiles.json b/cmd/cli/app/artifact/fixture/mock_list_profiles.json new file mode 100644 index 0000000000..b18e2af767 --- /dev/null +++ b/cmd/cli/app/artifact/fixture/mock_list_profiles.json @@ -0,0 +1,7 @@ +{ + "profiles": [ + { + "name": "artifact-security-baseline" + } + ] +} \ No newline at end of file diff --git a/cmd/cli/app/artifact/fixture/mock_profile_status.json b/cmd/cli/app/artifact/fixture/mock_profile_status.json new file mode 100644 index 0000000000..f9fbece4d9 --- /dev/null +++ b/cmd/cli/app/artifact/fixture/mock_profile_status.json @@ -0,0 +1,31 @@ +{ + "profile_status": { + "profile_id": "artifact-security-baseline", + "profile_name": "artifact-security-baseline", + "profile_status": "failure", + "last_updated": "2024-01-01T00:00:00Z" + }, + "rule_evaluation_status": [ + { + "profile_id": "artifact-security-baseline", + "rule_id": "artifact-attestation-slsa", + "entity": "artifact", + "status": "failure", + "last_updated": "2024-01-01T00:00:00Z", + "entity_info": { + "name": "owner-1/artifact-1" + }, + "details": "artifact attestation is disabled for this image", + "guidance": "enable artifact attestations before release", + "remediation_details": "rebuild the artifact with attestations enabled", + "remediation_url": "https://example.com/remediate/artifact-111", + "rule_type_name": "artifact_attestation_slsa", + "rule_description_name": "Require artifact attestation", + "alert": { + "status": "on", + "details": "artifact attestation alert is active", + "url": "https://example.com/alerts/artifact-111" + } + } + ] +} \ No newline at end of file diff --git a/cmd/cli/app/artifact/testdata/artifact_get.table.golden b/cmd/cli/app/artifact/testdata/artifact_get.table.golden new file mode 100644 index 0000000000..d6ac60079c --- /dev/null +++ b/cmd/cli/app/artifact/testdata/artifact_get.table.golden @@ -0,0 +1,17 @@ + ID │ TYPE │ OWNER │ NAME │ REPOSITORY │ VISIBILITY │ CREATION DATE +───────┼────────┼──────────┼──────────────┼──────────────┼──────────────┼─────────────────────────── + 111 │ image │ owner-1 │ artifact-1 │ org/repo │ public │ 2024-01-02T15:04:05Z + PROFILE │ RULE │ RESULT │ DETAILS +────────────────────────────┼──────────────────────────────┼─────────┼────────────────────────────── + artifact-security-baseline │ Require artifact attestation │ failure │ Alert: artifact attestation + │ │ │ alert is active URL: + │ │ │ https://example.com/alerts/a + │ │ │ rtifact-111 Remediation: + │ │ │ rebuild the artifact with + │ │ │ attestations enabled URL: + │ │ │ https://example.com/remediat + │ │ │ e/artifact-111 Details: + │ │ │ artifact attestation is + │ │ │ disabled for this image + │ │ │ Guidance: enable artifact + │ │ │ attestations before release diff --git a/cmd/cli/app/artifact/testdata/artifact_get.yaml.golden b/cmd/cli/app/artifact/testdata/artifact_get.yaml.golden new file mode 100644 index 0000000000..d57865a270 --- /dev/null +++ b/cmd/cli/app/artifact/testdata/artifact_get.yaml.golden @@ -0,0 +1,27 @@ +artifact: + artifactPk: "111" + createdAt: "2024-01-02T15:04:05Z" + name: artifact-1 + owner: owner-1 + repository: org/repo + type: image + visibility: public + +- alert: + details: artifact attestation alert is active + status: "on" + url: https://example.com/alerts/artifact-111 + details: artifact attestation is disabled for this image + entity: artifact + entityInfo: + name: owner-1/artifact-1 + guidance: enable artifact attestations before release + lastUpdated: "2024-01-01T00:00:00Z" + profileId: artifact-security-baseline + remediationDetails: rebuild the artifact with attestations enabled + remediationUrl: https://example.com/remediate/artifact-111 + ruleDescriptionName: Require artifact attestation + ruleId: artifact-attestation-slsa + ruleTypeName: artifact_attestation_slsa + status: failure + diff --git a/cmd/cli/app/profile/status/status_get_test.go b/cmd/cli/app/profile/status/status_get_test.go index 6af02d9e7b..7d22ab1f6e 100644 --- a/cmd/cli/app/profile/status/status_get_test.go +++ b/cmd/cli/app/profile/status/status_get_test.go @@ -80,24 +80,6 @@ func TestStatusGetCommand(t *testing.T) { }, GoldenFileName: "status_get_uuid_entity.txt", }, - { - Name: "status get json output", - Args: []string{"profile", "status", "get", "-i", testId, "-e", testEntityName, "-t", testEntityType, "-o", "json"}, - MockSetup: func(t *testing.T, ctrl *gomock.Controller) context.Context { - t.Helper() - client := mockv1.NewMockProfileServiceClient(ctrl) - - mockResp := &minderv1.GetProfileStatusByIdResponse{} - cli.LoadFixture(t, "mock_profile_status.json", mockResp) - - client.EXPECT(). - GetProfileStatusById(gomock.Any(), gomock.Any()). - Return(mockResp, nil) - - return cli.WithRPCClient[minderv1.ProfileServiceClient](context.Background(), client) - }, - GoldenFileName: "status_get.json", - }, { Name: "status get yaml output", Args: []string{"profile", "status", "get", "-n", testName, "-e", testEntityName, "-t", testEntityType, "-o", "yaml"}, diff --git a/cmd/cli/app/profile/status/status_list_test.go b/cmd/cli/app/profile/status/status_list_test.go index cb2645d726..c4047bc979 100644 --- a/cmd/cli/app/profile/status/status_list_test.go +++ b/cmd/cli/app/profile/status/status_list_test.go @@ -130,24 +130,6 @@ func TestStatusListCommand(t *testing.T) { }, GoldenFileName: "status_list_filter_ruletype.txt", }, - { - Name: "status list json success", - Args: []string{"profile", "status", "list", "-n", testName, "-o", "json"}, - MockSetup: func(t *testing.T, ctrl *gomock.Controller) context.Context { - t.Helper() - client := mockv1.NewMockProfileServiceClient(ctrl) - - mockResp := &minderv1.GetProfileStatusByNameResponse{} - cli.LoadFixture(t, "mock_profile_status.json", mockResp) - - client.EXPECT(). - GetProfileStatusByName(gomock.Any(), gomock.Any()). - Return(mockResp, nil) - - return cli.WithRPCClient[minderv1.ProfileServiceClient](context.Background(), client) - }, - GoldenFileName: "status_list.json", - }, { Name: "status list yaml success", Args: []string{"profile", "status", "list", "-n", testName, "-o", "yaml"}, diff --git a/cmd/cli/app/profile/status/testdata/status_get.json.golden b/cmd/cli/app/profile/status/testdata/status_get.json.golden deleted file mode 100644 index fb2c2c9253..0000000000 --- a/cmd/cli/app/profile/status/testdata/status_get.json.golden +++ /dev/null @@ -1,48 +0,0 @@ -{ - "profileStatus": { - "profileId": "11111111-1111-1111-1111-111111111111", - "profileName": "mock-profile", - "profileStatus": "success", - "lastUpdated": "2024-01-01T00:00:00Z" - }, - "ruleEvaluationStatus": [ - { - "profileId": "11111111-1111-1111-1111-111111111111", - "ruleId": "mock-rule-123", - "ruleName": "secret_scanning", - "entity": "repository", - "status": "success", - "lastUpdated": "2024-01-01T00:00:00Z", - "entityInfo": { - "entity_type": "repository", - "name": "acme-corp/mock-repo", - "provider": "github-app-mock-provider", - "repo_name": "mock-repo", - "repo_owner": "acme-corp", - "repository_id": "22222222-2222-2222-2222-222222222222" - }, - "details": "Mock rule evaluation succeeded.", - "ruleTypeName": "secret_scanning", - "ruleDisplayName": "Enable secret scanning to detect hardcoded secrets" - }, - { - "profileId": "11111111-1111-1111-1111-111111111111", - "ruleId": "mock-rule-456", - "ruleName": "codeql_enabled", - "entity": "repository", - "status": "failure", - "lastUpdated": "2024-01-01T00:00:00Z", - "entityInfo": { - "entity_type": "repository", - "name": "acme-corp/mock-repo", - "provider": "github-app-mock-provider", - "repo_name": "mock-repo", - "repo_owner": "acme-corp", - "repository_id": "22222222-2222-2222-2222-222222222222" - }, - "details": "Mock rule evaluation failed.", - "ruleTypeName": "codeql_enabled", - "ruleDisplayName": "Enable CodeQL for vulnerability scanning" - } - ] -} diff --git a/cmd/cli/app/profile/status/testdata/status_list.json.golden b/cmd/cli/app/profile/status/testdata/status_list.json.golden deleted file mode 100644 index fb2c2c9253..0000000000 --- a/cmd/cli/app/profile/status/testdata/status_list.json.golden +++ /dev/null @@ -1,48 +0,0 @@ -{ - "profileStatus": { - "profileId": "11111111-1111-1111-1111-111111111111", - "profileName": "mock-profile", - "profileStatus": "success", - "lastUpdated": "2024-01-01T00:00:00Z" - }, - "ruleEvaluationStatus": [ - { - "profileId": "11111111-1111-1111-1111-111111111111", - "ruleId": "mock-rule-123", - "ruleName": "secret_scanning", - "entity": "repository", - "status": "success", - "lastUpdated": "2024-01-01T00:00:00Z", - "entityInfo": { - "entity_type": "repository", - "name": "acme-corp/mock-repo", - "provider": "github-app-mock-provider", - "repo_name": "mock-repo", - "repo_owner": "acme-corp", - "repository_id": "22222222-2222-2222-2222-222222222222" - }, - "details": "Mock rule evaluation succeeded.", - "ruleTypeName": "secret_scanning", - "ruleDisplayName": "Enable secret scanning to detect hardcoded secrets" - }, - { - "profileId": "11111111-1111-1111-1111-111111111111", - "ruleId": "mock-rule-456", - "ruleName": "codeql_enabled", - "entity": "repository", - "status": "failure", - "lastUpdated": "2024-01-01T00:00:00Z", - "entityInfo": { - "entity_type": "repository", - "name": "acme-corp/mock-repo", - "provider": "github-app-mock-provider", - "repo_name": "mock-repo", - "repo_owner": "acme-corp", - "repository_id": "22222222-2222-2222-2222-222222222222" - }, - "details": "Mock rule evaluation failed.", - "ruleTypeName": "codeql_enabled", - "ruleDisplayName": "Enable CodeQL for vulnerability scanning" - } - ] -} diff --git a/cmd/cli/app/profile/status/testdata/status_list_table_detailed.txt.golden b/cmd/cli/app/profile/status/testdata/status_list_table_detailed.txt.golden index 7053446cba..6600fe1817 100644 --- a/cmd/cli/app/profile/status/testdata/status_list_table_detailed.txt.golden +++ b/cmd/cli/app/profile/status/testdata/status_list_table_detailed.txt.golden @@ -2,10 +2,10 @@ ───────────────────────────────┼────────────────┼─────────────────────────────────────────────────── mock-profile │ ✅ │ 2024-01-01T00:00:00Z - ENTITY │ RULE NAME │ STATUS │ DETAILS -─────────────────────────┼──────────────────────┼─────────┼───────────────────────────────────────── - acme-corp/mock-repo │ │ ✅ │ Mock rule evaluation succeeded. - [repository] │ [secret_scanning] │ │ - ├──────────────────────┼─────────┼───────────────────────────────────────── - │ │ ⛔ │ Mock rule evaluation failed. - │ [codeql_enabled] │ │ + ENTITY │ RULE │ RESULT │ DETAILS +─────────────────────┼────────────────────────────────────────┼────────┼──────────────────────────── + acme-corp/mock-repo │ Enable secret scanning to detect │ ✅ │ Details: Mock rule + [repository] │ hardcoded secrets │ │ evaluation succeeded. + ├────────────────────────────────────────┼────────┼──────────────────────────── + │ Enable CodeQL for vulnerability │ ⛔ │ Details: Mock rule + │ scanning │ │ evaluation failed. diff --git a/cmd/cli/app/profile/status/testdata/status_list_table_detailed_no_emoji.txt.golden b/cmd/cli/app/profile/status/testdata/status_list_table_detailed_no_emoji.txt.golden index 6120a6ee62..880b023a38 100644 --- a/cmd/cli/app/profile/status/testdata/status_list_table_detailed_no_emoji.txt.golden +++ b/cmd/cli/app/profile/status/testdata/status_list_table_detailed_no_emoji.txt.golden @@ -2,10 +2,10 @@ ───────────────────────────────┼────────────────┼─────────────────────────────────────────────────── mock-profile │ Ok │ 2024-01-01T00:00:00Z - ENTITY │ RULE NAME │ STATUS │ DETAILS -─────────────────────────┼──────────────────────┼─────────┼───────────────────────────────────────── - acme-corp/mock-repo │ │ Ok │ Mock rule evaluation succeeded. - [repository] │ [secret_scanning] │ │ - ├──────────────────────┼─────────┼───────────────────────────────────────── - │ │ Failed │ Mock rule evaluation failed. - │ [codeql_enabled] │ │ + ENTITY │ RULE │ RESULT │ DETAILS +─────────────────────┼────────────────────────────────────────┼────────┼──────────────────────────── + acme-corp/mock-repo │ Enable secret scanning to detect │ Ok │ Details: Mock rule + [repository] │ hardcoded secrets │ │ evaluation succeeded. + ├────────────────────────────────────────┼────────┼──────────────────────────── + │ Enable CodeQL for vulnerability │ Failed │ Details: Mock rule + │ scanning │ │ evaluation failed. diff --git a/cmd/cli/app/profile/table_render.go b/cmd/cli/app/profile/table_render.go index 1727fd83ca..a63b4132c0 100644 --- a/cmd/cli/app/profile/table_render.go +++ b/cmd/cli/app/profile/table_render.go @@ -4,6 +4,7 @@ package profile import ( + "cmp" "fmt" "io" "slices" @@ -11,8 +12,8 @@ import ( "time" "google.golang.org/protobuf/types/known/structpb" - "gopkg.in/yaml.v2" + "github.com/mindersec/minder/internal/util" "github.com/mindersec/minder/internal/util/cli/table" "github.com/mindersec/minder/internal/util/cli/table/layouts" "github.com/mindersec/minder/internal/util/cli/types" @@ -24,12 +25,11 @@ func marshalStructOrEmpty(v *structpb.Struct) string { return "" } - m := v.AsMap() - out, err := yaml.Marshal(m) + out, err := util.GetYamlFromProto(v) if err != nil { return "" } - return string(out) + return strings.TrimSpace(out) } // NewProfileSettingsTable creates a new table for rendering profile settings @@ -94,11 +94,65 @@ func RenderProfileStatusTable(ps *minderv1.ProfileStatus, t table.Table, emoji b // NewRuleEvaluationsTable creates a new table for rendering rule evaluations func NewRuleEvaluationsTable(out io.Writer) table.Table { return table.New(table.Simple, layouts.Default, out, - []string{"Entity", "Rule Name", "Status", "Details"}). + []string{"Entity", "Rule", "Result", "Details"}). SetAutoMerge(true). SetEqualColumns(false) } +func buildLabeledBlock(label, detail, url string) string { + detail = strings.TrimSpace(detail) + url = strings.TrimSpace(url) + + if detail == "" && url == "" { + return "" + } + + if detail == "" { + return fmt.Sprintf("%s: %s", label, url) + } + + block := fmt.Sprintf("%s: %s", label, detail) + if url != "" { + block += fmt.Sprintf("\nURL: %s", url) + } + + return block +} + +func buildDetailSummary(eval *minderv1.RuleEvaluationStatus) string { + sections := make([]string, 0, 4) + + if alert := eval.GetAlert(); alert != nil { + if section := buildLabeledBlock("Alert", alert.GetDetails(), alert.GetUrl()); section != "" { + sections = append(sections, section) + } + } + + if section := buildLabeledBlock("Remediation", eval.GetRemediationDetails(), eval.GetRemediationUrl()); section != "" { + sections = append(sections, section) + } + + if detail := strings.TrimSpace(eval.GetDetails()); detail != "" { + sections = append(sections, fmt.Sprintf("Details: %s", detail)) + } + + if guidance := strings.TrimSpace(eval.GetGuidance()); guidance != "" { + sections = append(sections, fmt.Sprintf("Guidance: %s", guidance)) + } + + return strings.Join(sections, "\n") +} + +// RuleDisplayName returns the most user-friendly rule name available. +func RuleDisplayName(eval *minderv1.RuleEvaluationStatus) string { + return strings.TrimSpace(cmp.Or(eval.GetRuleDescriptionName(), eval.GetRuleDisplayName(), eval.GetRuleTypeName())) +} + +// FormatEvaluationReasoning returns a rendered reasoning block for CLI output. +func FormatEvaluationReasoning(eval *minderv1.RuleEvaluationStatus) string { + return cmp.Or(buildDetailSummary(eval), "-") +} + // RenderRuleEvaluationStatusTable renders the rule evaluations table. func RenderRuleEvaluationStatusTable( statuses []*minderv1.RuleEvaluationStatus, @@ -114,11 +168,13 @@ func RenderRuleEvaluationStatusTable( for _, eval := range statuses { evalInfo := types.RuleEvalStatus(eval) + ruleName := RuleDisplayName(eval) + reasoning := FormatEvaluationReasoning(eval) t.AddRowWithColor( layouts.NoColor(fmt.Sprintf("%s\n[%s]", eval.EntityInfo["name"], eval.Entity)), - layouts.NoColor(fmt.Sprintf("%s\n[%s]", eval.RuleDescriptionName, eval.RuleTypeName)), + layouts.NoColor(ruleName), table.GetStatusIcon(evalInfo, emoji), - table.BestDetail(evalInfo), + layouts.NoColor(reasoning), ) } } diff --git a/cmd/cli/app/profile/table_render_test.go b/cmd/cli/app/profile/table_render_test.go new file mode 100644 index 0000000000..1118939471 --- /dev/null +++ b/cmd/cli/app/profile/table_render_test.go @@ -0,0 +1,39 @@ +// SPDX-FileCopyrightText: Copyright 2026 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package profile + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" +) + +func TestFormatEvaluationReasoning(t *testing.T) { + t.Parallel() + + eval := &minderv1.RuleEvaluationStatus{ + Details: "the base check failed", + Guidance: "rebuild with the required settings", + RemediationDetails: "apply the remediation workflow", + RemediationUrl: "https://example.com/remediate", + RuleDescriptionName: "Require artifact attestation", + RuleTypeName: "artifact_attestation_slsa", + Status: "failure", + Alert: &minderv1.EvalResultAlert{ + Status: "on", + Details: "alert is enabled", + Url: "https://example.com/alert", + }, + } + + assert.Equal(t, "Alert: alert is enabled\nURL: https://example.com/alert\nRemediation: apply the remediation workflow\nURL: https://example.com/remediate\nDetails: the base check failed\nGuidance: rebuild with the required settings", FormatEvaluationReasoning(eval)) +} + +func TestFormatEvaluationReasoning_Empty(t *testing.T) { + t.Parallel() + + assert.Equal(t, "-", FormatEvaluationReasoning(&minderv1.RuleEvaluationStatus{})) +} diff --git a/cmd/cli/app/profile/testdata/apply_create.table.golden b/cmd/cli/app/profile/testdata/apply_create.table.golden index 439eb3874c..5c7885c402 100644 --- a/cmd/cli/app/profile/testdata/apply_create.table.golden +++ b/cmd/cli/app/profile/testdata/apply_create.table.golden @@ -7,7 +7,6 @@ Successfully created new profile named: mock-attestation-profile │ │ │ schedule_interval: weekly ├───────────────────────────┼──────────────────────────┼──────────────────────────────── │ branch_protection_enabled │ branch: "" │ - │ │ │ ────────────┼───────────────────────────┼──────────────────────────┼──────────────────────────────── artifact │ artifact_attestation_slsa │ name: fake-artifact-name │ event: - workflow_dispatch │ │ tags: - mock-tag │ runner_environment: diff --git a/cmd/cli/app/profile/testdata/apply_multiple.table.golden b/cmd/cli/app/profile/testdata/apply_multiple.table.golden index 3eb3fb0fe8..bdd5d5d66e 100644 --- a/cmd/cli/app/profile/testdata/apply_multiple.table.golden +++ b/cmd/cli/app/profile/testdata/apply_multiple.table.golden @@ -7,7 +7,6 @@ Successfully created new profile named: mock-attestation-profile │ │ │ schedule_interval: weekly ├───────────────────────────┼──────────────────────────┼──────────────────────────────── │ branch_protection_enabled │ branch: "" │ - │ │ │ ────────────┼───────────────────────────┼──────────────────────────┼──────────────────────────────── artifact │ artifact_attestation_slsa │ name: fake-artifact-name │ event: - workflow_dispatch │ │ tags: - mock-tag │ runner_environment: @@ -27,5 +26,4 @@ Successfully updated existing profile named: mock-dependabot-profile │ │ │ schedule_interval: weekly ├──────────────────────────────┼──────────────┼──────────────────────────────────────── │ branch_protection_enabled │ branch: "" │ - │ │ │ diff --git a/cmd/cli/app/profile/testdata/apply_update.table.golden b/cmd/cli/app/profile/testdata/apply_update.table.golden index 5566968437..90677f1755 100644 --- a/cmd/cli/app/profile/testdata/apply_update.table.golden +++ b/cmd/cli/app/profile/testdata/apply_update.table.golden @@ -6,5 +6,4 @@ Successfully updated existing profile named: mock-dependabot-profile │ │ │ schedule_interval: weekly ├──────────────────────────────┼──────────────┼──────────────────────────────────────── │ branch_protection_enabled │ branch: "" │ - │ │ │ diff --git a/cmd/cli/app/profile/testdata/create_success.table.golden b/cmd/cli/app/profile/testdata/create_success.table.golden index a760374a90..7aa4b66013 100644 --- a/cmd/cli/app/profile/testdata/create_success.table.golden +++ b/cmd/cli/app/profile/testdata/create_success.table.golden @@ -7,7 +7,6 @@ Successfully created new profile named: mock-attestation-profile │ │ │ schedule_interval: weekly ├───────────────────────────┼──────────────────────────┼──────────────────────────────── │ branch_protection_enabled │ branch: "" │ - │ │ │ ────────────┼───────────────────────────┼──────────────────────────┼──────────────────────────────── artifact │ artifact_attestation_slsa │ name: fake-artifact-name │ event: - workflow_dispatch │ │ tags: - mock-tag │ runner_environment: diff --git a/cmd/cli/app/profile/testdata/get_by_name_table.txt.golden b/cmd/cli/app/profile/testdata/get_by_name_table.txt.golden index a2bc9d8602..c34672359f 100644 --- a/cmd/cli/app/profile/testdata/get_by_name_table.txt.golden +++ b/cmd/cli/app/profile/testdata/get_by_name_table.txt.golden @@ -5,4 +5,3 @@ ENTITY │ RULE │ RULE PARAMS │ RULE DEFINITION ───────────────┼──────────────────────────────┼────────────────┼──────────────────────────────────── repository │ dependabot_configured │ │ package_ecosystem: gomod - │ │ │ diff --git a/go.mod b/go.mod index c0b5459c6c..474bcfb973 100644 --- a/go.mod +++ b/go.mod @@ -101,7 +101,6 @@ require ( google.golang.org/grpc v1.80.0 google.golang.org/protobuf v1.36.11 gopkg.in/op/go-logging.v1 v1.0.0-20160211212156-b2cb9fa56473 - gopkg.in/yaml.v2 v2.4.0 gopkg.in/yaml.v3 v3.0.1 k8s.io/apimachinery v0.35.1 k8s.io/client-go v0.35.1 diff --git a/internal/util/cli/cli.go b/internal/util/cli/cli.go index d6f7e25bff..6fc8742df0 100644 --- a/internal/util/cli/cli.go +++ b/internal/util/cli/cli.go @@ -90,7 +90,6 @@ func GRPCClientWrapRunE( ctx, cancel := GetAppContext(cmd.Context(), viper.GetViper()) defer cancel() - defer c.Close() return runEFunc(ctx, cmd, args, c) diff --git a/internal/util/cli/context.go b/internal/util/cli/context.go index 98fd7bf95c..7a04fd7dca 100644 --- a/internal/util/cli/context.go +++ b/internal/util/cli/context.go @@ -16,7 +16,7 @@ type rpcKey struct { clientType reflect.Type } -// WithRPCClient injects the provided RPC client into the context, +// WithRPCClient injects the provided RPC client into the context. func WithRPCClient[T any](ctx context.Context, client T) context.Context { key := rpcKey{clientType: reflect.TypeOf((*T)(nil)).Elem()} return context.WithValue(ctx, key, client) @@ -29,12 +29,12 @@ func GetRPCClient[T any](ctx context.Context) (T, bool) { return client, ok } -// Cleanup is a function type used to define a routine that releases resources +// Cleanup is a function type used to define a routine that releases resources. type Cleanup = func() // GetCLIClient takes a factory for a GRPC client service and returns -// a client, a cleanup function to close the connection and an error -func GetCLIClient[T interface{}](cmd *cobra.Command, client func(grpc.ClientConnInterface) T) (T, Cleanup, error) { +// a client, a cleanup function to close the connection and an error. +func GetCLIClient[T any](cmd *cobra.Command, client func(grpc.ClientConnInterface) T) (T, Cleanup, error) { var empty T ctx, cancel := GetAppContext(cmd.Context(), viper.GetViper()) diff --git a/internal/util/cli/table/common.go b/internal/util/cli/table/common.go index 673b7eb32a..c0cb9dd4ce 100644 --- a/internal/util/cli/table/common.go +++ b/internal/util/cli/table/common.go @@ -165,22 +165,3 @@ func GetStatusIcon(eval EvalStatus, emoji bool) layouts.ColoredColumn { return colorFunc(strings.Join(tokens, separator)) } - -// BestDetail returns the best detail for the given evaluation status. -func BestDetail(eval EvalStatus) layouts.ColoredColumn { - // TODO: combine with GetStatusIcon, and pick color, etc based on status - icon := GetStatusIcon(eval, false) - - detailText := eval.GetStatusDetail() - if eval.GetRemediationDetail() != "" { - detailText = eval.GetRemediationDetail() - } else if eval.GetAlert().GetDetails() != "" { - detailText = eval.GetAlert().GetDetails() - } - - // Return the detail text with the same color as the status icon - return layouts.ColoredColumn{ - Column: detailText, - Color: icon.Color, - } -} diff --git a/pkg/api/protobuf/go/minder/v1/mock/mock_artifact.go b/pkg/api/protobuf/go/minder/v1/mock/mock_artifact.go new file mode 100644 index 0000000000..db051b9c02 --- /dev/null +++ b/pkg/api/protobuf/go/minder/v1/mock/mock_artifact.go @@ -0,0 +1,103 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1 (interfaces: ArtifactServiceClient) +// +// Generated by this command: +// +// mockgen -package mock_minderv1 -destination pkg/api/protobuf/go/minder/v1/mock/mock_artifact.go github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1 ArtifactServiceClient +// + +// Package mock_minderv1 is a generated GoMock package. +package mock + +import ( + context "context" + reflect "reflect" + + v1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" + gomock "go.uber.org/mock/gomock" + grpc "google.golang.org/grpc" +) + +// MockArtifactServiceClient is a mock of ArtifactServiceClient interface. +type MockArtifactServiceClient struct { + ctrl *gomock.Controller + recorder *MockArtifactServiceClientMockRecorder + isgomock struct{} +} + +// MockArtifactServiceClientMockRecorder is the mock recorder for MockArtifactServiceClient. +type MockArtifactServiceClientMockRecorder struct { + mock *MockArtifactServiceClient +} + +// NewMockArtifactServiceClient creates a new mock instance. +func NewMockArtifactServiceClient(ctrl *gomock.Controller) *MockArtifactServiceClient { + mock := &MockArtifactServiceClient{ctrl: ctrl} + mock.recorder = &MockArtifactServiceClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockArtifactServiceClient) EXPECT() *MockArtifactServiceClientMockRecorder { + return m.recorder +} + +// GetArtifactById mocks base method. +func (m *MockArtifactServiceClient) GetArtifactById(ctx context.Context, in *v1.GetArtifactByIdRequest, opts ...grpc.CallOption) (*v1.GetArtifactByIdResponse, error) { + m.ctrl.T.Helper() + varargs := []any{ctx, in} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetArtifactById", varargs...) + ret0, _ := ret[0].(*v1.GetArtifactByIdResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetArtifactById indicates an expected call of GetArtifactById. +func (mr *MockArtifactServiceClientMockRecorder) GetArtifactById(ctx, in any, opts ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{ctx, in}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetArtifactById", reflect.TypeOf((*MockArtifactServiceClient)(nil).GetArtifactById), varargs...) +} + +// GetArtifactByName mocks base method. +func (m *MockArtifactServiceClient) GetArtifactByName(ctx context.Context, in *v1.GetArtifactByNameRequest, opts ...grpc.CallOption) (*v1.GetArtifactByNameResponse, error) { + m.ctrl.T.Helper() + varargs := []any{ctx, in} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetArtifactByName", varargs...) + ret0, _ := ret[0].(*v1.GetArtifactByNameResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetArtifactByName indicates an expected call of GetArtifactByName. +func (mr *MockArtifactServiceClientMockRecorder) GetArtifactByName(ctx, in any, opts ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{ctx, in}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetArtifactByName", reflect.TypeOf((*MockArtifactServiceClient)(nil).GetArtifactByName), varargs...) +} + +// ListArtifacts mocks base method. +func (m *MockArtifactServiceClient) ListArtifacts(ctx context.Context, in *v1.ListArtifactsRequest, opts ...grpc.CallOption) (*v1.ListArtifactsResponse, error) { + m.ctrl.T.Helper() + varargs := []any{ctx, in} + for _, a := range opts { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "ListArtifacts", varargs...) + ret0, _ := ret[0].(*v1.ListArtifactsResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListArtifacts indicates an expected call of ListArtifacts. +func (mr *MockArtifactServiceClientMockRecorder) ListArtifacts(ctx, in any, opts ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{ctx, in}, opts...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListArtifacts", reflect.TypeOf((*MockArtifactServiceClient)(nil).ListArtifacts), varargs...) +}