Re-applies the pending status change and uses a transaction for 00004…

…1 migration (#2865) * Reapply "Add a pending remediation status and support tracking opened PRs" (#2862) This reverts commit be7bbf3. * Make migration 000041 happen in a single transaction Signed-off-by: Radoslav Dimitrov <[email protected]> --------- Signed-off-by: Radoslav Dimitrov <[email protected]>
mindersec · Mar 29, 2024 · b7b4f82 · b7b4f82
1 parent be7bbf3
commit b7b4f82
Show file tree

Hide file tree

Showing 19 changed files with 578 additions and 313 deletions.
diff --git a/cmd/cli/app/profile/table_render.go b/cmd/cli/app/profile/table_render.go
@@ -189,7 +189,7 @@ func getEvalStatusText(status string) string {
 
 // Gets a friendly status text with an emoji
 func getRemediationStatusText(status string) string {
-	// remediation statuses can be 'success', 'failure', 'error', 'skipped', 'not supported'
+	// remediation statuses can be 'success', 'failure', 'error', 'skipped', 'pending' or 'not supported'
 	switch strings.ToLower(status) {
 	case successStatus:
 		return "Success"
@@ -199,6 +199,8 @@ func getRemediationStatusText(status string) string {
 		return "Error"
 	case skippedStatus:
 		return "Skipped" // visually empty as we didn't have to remediate
+	case pendingStatus:
+		return "Pending"
 	case notAvailableStatus:
 		return "Not Available"
 	default:

diff --git a/database/migrations/000041_remediate_metadata.down.sql b/database/migrations/000041_remediate_metadata.down.sql
@@ -0,0 +1,17 @@
+-- Copyright 2024 Stacklok, Inc
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--      http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+ALTER TABLE rule_details_remediate DROP COLUMN metadata;
+
+-- It is not possible to drop added values from enums, ref. `pending` for remediation_status_types
diff --git a/database/migrations/000041_remediate_metadata.up.sql b/database/migrations/000041_remediate_metadata.up.sql
@@ -0,0 +1,21 @@
+-- Copyright 2024 Stacklok, Inc
+--
+-- Licensed under the Apache License, Version 2.0 (the "License");
+-- you may not use this file except in compliance with the License.
+-- You may obtain a copy of the License at
+--
+--      http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+
+BEGIN;
+
+ALTER TABLE rule_details_remediate ADD COLUMN metadata JSONB NOT NULL DEFAULT '{}';
+
+ALTER TYPE remediation_status_types ADD VALUE 'pending';
+
+COMMIT;
diff --git a/database/query/profile_status.sql b/database/query/profile_status.sql
@@ -28,13 +28,15 @@ INSERT INTO rule_details_remediate (
     rule_eval_id,
     status,
     details,
+    metadata,
     last_updated
 )
-VALUES ($1, $2, $3, NOW())
+VALUES ($1, $2, $3, sqlc.arg(metadata)::jsonb, NOW())
 ON CONFLICT(rule_eval_id)
     DO UPDATE SET
                   status = $2,
                   details = $3,
+                  metadata = sqlc.arg(metadata)::jsonb,
                   last_updated = NOW()
     WHERE rule_details_remediate.rule_eval_id = $1
 RETURNING id;
@@ -50,9 +52,9 @@ INSERT INTO rule_details_alert (
 VALUES ($1, $2, $3, sqlc.arg(metadata)::jsonb, NOW())
 ON CONFLICT(rule_eval_id)
     DO UPDATE SET
-                  status = CASE WHEN $2 != 'skipped' THEN $2 ELSE rule_details_alert.status END,
-                  details = CASE WHEN $2 != 'skipped' THEN $3 ELSE rule_details_alert.details END,
-                  metadata = CASE WHEN $2 != 'skipped' THEN sqlc.arg(metadata)::jsonb ELSE rule_details_alert.metadata END,
+                  status = $2,
+                  details = $3,
+                  metadata = sqlc.arg(metadata)::jsonb,
                   last_updated = NOW()
     WHERE rule_details_alert.rule_eval_id = $1
 RETURNING id;
@@ -96,6 +98,7 @@ WITH
            rule_eval_id,
            status AS rem_status,
            details AS rem_details,
+           metadata AS rem_metadata,
            last_updated AS rem_last_updated
        FROM rule_details_remediate
    ),
@@ -115,6 +118,7 @@ SELECT
     ed.eval_details,
     rd.rem_status,
     rd.rem_details,
+    rd.rem_metadata,
     rd.rem_last_updated,
     ad.alert_status,
     ad.alert_details,

diff --git a/internal/db/models.go b/internal/db/models.go
diff --git a/internal/db/profile_status.sql.go b/internal/db/profile_status.sql.go
diff --git a/internal/db/profiles_test.go b/internal/db/profiles_test.go
@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+	"github.com/sqlc-dev/pqtype"
 	"github.com/stretchr/testify/require"
 
 	"github.com/stacklok/minder/internal/util/rand"
@@ -28,6 +29,10 @@ func createRandomProfile(t *testing.T, prov Provider, projectID uuid.UUID, label
 			ActionType: "on",
 			Valid:      true,
 		},
+		Alert: NullActionType{
+			ActionType: "on",
+			Valid:      true,
+		},
 		Labels: labels,
 	}
 
@@ -109,7 +114,7 @@ func upsertEvalStatus(
 
 func upsertRemediationStatus(
 	t *testing.T, profileID uuid.UUID, repoID uuid.UUID, ruleTypeID uuid.UUID,
-	remStatus RemediationStatusTypes, details string,
+	remStatus RemediationStatusTypes, details string, metadata json.RawMessage,
 ) {
 	t.Helper()
 
@@ -129,6 +134,7 @@ func upsertRemediationStatus(
 		RuleEvalID: id,
 		Status:     remStatus,
 		Details:    details,
+		Metadata:   metadata,
 	})
 	require.NoError(t, err)
 }
@@ -645,8 +651,10 @@ func compareRows(t *testing.T, a, b *ListRuleEvaluationsByProfileIdRow) {
 	require.Equal(t, a.EvalDetails, b.EvalDetails)
 	require.Equal(t, a.RemStatus, b.RemStatus)
 	require.Equal(t, a.RemDetails, b.RemDetails)
+	require.Equal(t, a.RemMetadata, b.RemMetadata)
 	require.Equal(t, a.AlertStatus, b.AlertStatus)
 	require.Equal(t, a.AlertDetails, b.AlertDetails)
+	require.Equal(t, a.AlertMetadata, b.AlertMetadata)
 	require.Equal(t, a.Entity, b.Entity)
 }
 
@@ -780,7 +788,7 @@ func TestListRuleEvaluations(t *testing.T) {
 					EvalStatusTypesFailure, "this rule failed")
 				upsertRemediationStatus(
 					t, profile.ID, randomEntities.repo.ID, randomEntities.ruleType1.ID,
-					RemediationStatusTypesSuccess, "this rule was remediated")
+					RemediationStatusTypesSuccess, "this rule was remediated", json.RawMessage(`{"pr_number": "56"}`))
 				upsertAlertStatus(
 					t, profile.ID, randomEntities.repo.ID, randomEntities.ruleType1.ID,
 					AlertStatusTypesOn, "we alerted about this rule", json.RawMessage(`{"ghsa_id": "GHSA-xxxx-xxxx-xxxx"}`))
@@ -806,6 +814,10 @@ func TestListRuleEvaluations(t *testing.T) {
 					String: "this rule was remediated",
 					Valid:  true,
 				},
+				RemMetadata: pqtype.NullRawMessage{
+					RawMessage: json.RawMessage(`{"pr_number": "56"}`),
+					Valid:      true,
+				},
 				AlertStatus: NullAlertStatusTypes{
 					AlertStatusTypes: AlertStatusTypesOn,
 					Valid:            true,
@@ -814,6 +826,10 @@ func TestListRuleEvaluations(t *testing.T) {
 					String: "we alerted about this rule",
 					Valid:  true,
 				},
+				AlertMetadata: pqtype.NullRawMessage{
+					RawMessage: json.RawMessage(`{"ghsa_id": "GHSA-xxxx-xxxx-xxxx"}`),
+					Valid:      true,
+				},
 				Entity: EntitiesRepository,
 			},
 		},