Change classifier terraform/YAML handling to accept friendly values (#67884)

* Accept and return booleans for Classifier action modes in tctl * Accept short risk levels and strict booleans for Classifier in tctl
2026-10-12 02:15:01 +00:00 · 2026-06-17 13:58:34 +00:00
parent 1ebcc101b7
commit a02aeaac3b
2 changed files with 267 additions and 3 deletions
@@ -18,11 +18,13 @@ package resources

 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"strings"

 	"github.com/gravitational/trace"
+	"google.golang.org/protobuf/encoding/protojson"

 	summarizerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/summarizer/v1"
 	"github.com/gravitational/teleport/api/types"
@@ -40,11 +42,28 @@ type classifierCollection []*summarizerv1.Classifier
 func (c classifierCollection) Resources() []types.Resource {
 	out := make([]types.Resource, 0, len(c))
 	for _, item := range c {
-		out = append(out, types.ProtoResource153ToLegacy(item))
+		out = append(out, classifierResource{
+			Resource:   types.ProtoResource153ToLegacy(item),
+			classifier: item,
+		})
 	}
 	return out
 }

+// classifierResource renders a Classifier's spec.actions in friendly form for "tctl get".
+type classifierResource struct {
+	types.Resource
+	classifier *summarizerv1.Classifier
+}
+
+func (r classifierResource) MarshalJSON() ([]byte, error) {
+	data, err := protojson.MarshalOptions{UseProtoNames: true}.Marshal(r.classifier)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	return classifierActionsToFriendly(data)
+}
+
 func (c classifierCollection) WriteText(w io.Writer, verbose bool) error {
 	headers := []string{"Name", "Description", "Kinds"}
 	var rows [][]string
@@ -85,8 +104,12 @@ func classifierHandler() Handler {
 func createClassifier(
 	ctx context.Context, clt *authclient.Client, raw services.UnknownResource, opts CreateOpts,
 ) error {
+	rawJSON, err := classifierActionsFromFriendly(raw.Raw)
+	if err != nil {
+		return trace.Wrap(err)
+	}
 	classifier, err := services.UnmarshalProtoResource[*summarizerv1.Classifier](
-		raw.Raw, services.DisallowUnknown())
+		rawJSON, services.DisallowUnknown())
 	if err != nil {
 		return trace.Wrap(err)
 	}
@@ -114,8 +137,12 @@ func createClassifier(
 func updateClassifier(
 	ctx context.Context, clt *authclient.Client, raw services.UnknownResource, opts CreateOpts,
 ) error {
+	rawJSON, err := classifierActionsFromFriendly(raw.Raw)
+	if err != nil {
+		return trace.Wrap(err)
+	}
 	classifier, err := services.UnmarshalProtoResource[*summarizerv1.Classifier](
-		raw.Raw, services.DisallowUnknown())
+		rawJSON, services.DisallowUnknown())
 	if err != nil {
 		return trace.Wrap(err)
 	}
@@ -174,3 +201,107 @@ func deleteClassifier(
 	fmt.Printf("classifier %q has been deleted\n", ref.Name)
 	return nil
 }
+
+const (
+	classifierActionModeEnabled  = "CLASSIFIER_ACTION_MODE_ENABLED"
+	classifierActionModeDisabled = "CLASSIFIER_ACTION_MODE_DISABLED"
+)
+
+var classifierActionFields = []string{"emit_audit_event", "flag_for_review", "risk_level_floor"}
+
+var riskLevelToShort = map[string]string{
+	"RISK_LEVEL_LOW":      "low",
+	"RISK_LEVEL_MEDIUM":   "medium",
+	"RISK_LEVEL_HIGH":     "high",
+	"RISK_LEVEL_CRITICAL": "critical",
+}
+
+var riskLevelFromShort = map[string]string{
+	"low":      "RISK_LEVEL_LOW",
+	"medium":   "RISK_LEVEL_MEDIUM",
+	"high":     "RISK_LEVEL_HIGH",
+	"critical": "RISK_LEVEL_CRITICAL",
+}
+
+// classifierActionsToFriendly converts spec.actions enums to booleans and a short risk level.
+func classifierActionsToFriendly(data []byte) ([]byte, error) {
+	return rewriteClassifierActions(data, func(field string, v any) (any, error) {
+		switch field {
+		case "emit_audit_event", "flag_for_review":
+			switch v {
+			case classifierActionModeEnabled:
+				return true, nil
+			case classifierActionModeDisabled:
+				return false, nil
+			}
+		case "risk_level_floor":
+			if s, ok := v.(string); ok {
+				if short, ok := riskLevelToShort[s]; ok {
+					return short, nil
+				}
+			}
+		}
+		return v, nil
+	})
+}
+
+// classifierActionsFromFriendly converts spec.actions booleans and short risk levels back to enums, rejecting any other value.
+func classifierActionsFromFriendly(data []byte) ([]byte, error) {
+	return rewriteClassifierActions(data, func(field string, v any) (any, error) {
+		switch field {
+		case "emit_audit_event", "flag_for_review":
+			b, ok := v.(bool)
+			if !ok {
+				return nil, trace.BadParameter("spec.actions.%s must be true or false", field)
+			}
+			if b {
+				return classifierActionModeEnabled, nil
+			}
+			return classifierActionModeDisabled, nil
+		case "risk_level_floor":
+			if s, ok := v.(string); ok {
+				if enum, ok := riskLevelFromShort[strings.ToLower(s)]; ok {
+					return enum, nil
+				}
+			}
+			return nil, trace.BadParameter(
+				`spec.actions.risk_level_floor must be one of "low", "medium", "high", "critical"`)
+		}
+		return v, nil
+	})
+}
+
+func rewriteClassifierActions(data []byte, convert func(field string, v any) (any, error)) ([]byte, error) {
+	var root map[string]any
+	if err := json.Unmarshal(data, &root); err != nil {
+		return nil, trace.Wrap(err)
+	}
+	spec, ok := root["spec"].(map[string]any)
+	if !ok {
+		return data, nil
+	}
+	actions, ok := spec["actions"].(map[string]any)
+	if !ok {
+		return data, nil
+	}
+	changed := false
+	for _, field := range classifierActionFields {
+		v, ok := actions[field]
+		if !ok {
+			continue
+		}
+		nv, err := convert(field, v)
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
+		if nv != v {
+			actions[field] = nv
+			changed = true
+		}
+	}
+	if !changed {
+		return data, nil
+	}
+	out, err := json.Marshal(root)
+	return out, trace.Wrap(err)
+}
@@ -0,0 +1,133 @@
+// Teleport
+// Copyright (C) 2026 Gravitational, Inc.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package resources
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	summarizerv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/summarizer/v1"
+	"github.com/gravitational/teleport/api/types/summarizer"
+	"github.com/gravitational/teleport/lib/services"
+)
+
+func makeClassifierWithActions(actions *summarizerv1.ClassifierActions) *summarizerv1.Classifier {
+	return summarizer.NewClassifier("test", summarizerv1.ClassifierSpec_builder{
+		Kinds:    []string{"ssh"},
+		Criteria: "test criteria",
+		Actions:  actions,
+	}.Build())
+}
+
+// "tctl get" renders toggles as booleans and risk_level_floor as a short string.
+func TestClassifierActionsMarshalFriendly(t *testing.T) {
+	c := makeClassifierWithActions(summarizerv1.ClassifierActions_builder{
+		EmitAuditEvent: summarizerv1.ClassifierActionMode_CLASSIFIER_ACTION_MODE_ENABLED,
+		FlagForReview:  summarizerv1.ClassifierActionMode_CLASSIFIER_ACTION_MODE_DISABLED,
+		RiskLevelFloor: summarizerv1.RiskLevel_RISK_LEVEL_HIGH,
+	}.Build())
+
+	data, err := classifierResource{classifier: c}.MarshalJSON()
+	require.NoError(t, err)
+
+	actions := unmarshalActions(t, data)
+	require.Equal(t, true, actions["emit_audit_event"])
+	require.Equal(t, false, actions["flag_for_review"])
+	require.Equal(t, "high", actions["risk_level_floor"])
+}
+
+func TestClassifierActionsUnspecifiedOmitted(t *testing.T) {
+	c := makeClassifierWithActions(summarizerv1.ClassifierActions_builder{
+		EmitAuditEvent: summarizerv1.ClassifierActionMode_CLASSIFIER_ACTION_MODE_ENABLED,
+		// flag_for_review and risk_level_floor are left unspecified.
+	}.Build())
+
+	data, err := classifierResource{classifier: c}.MarshalJSON()
+	require.NoError(t, err)
+
+	actions := unmarshalActions(t, data)
+	require.Equal(t, true, actions["emit_audit_event"])
+	_, hasFlag := actions["flag_for_review"]
+	require.False(t, hasFlag, "an unspecified toggle should be omitted, not printed")
+	_, hasRisk := actions["risk_level_floor"]
+	require.False(t, hasRisk, "an unspecified risk level should be omitted, not printed")
+}
+
+// Friendly values survive a get -> create round-trip across every risk level.
+func TestClassifierActionsRoundTrip(t *testing.T) {
+	for _, level := range []summarizerv1.RiskLevel{
+		summarizerv1.RiskLevel_RISK_LEVEL_LOW,
+		summarizerv1.RiskLevel_RISK_LEVEL_MEDIUM,
+		summarizerv1.RiskLevel_RISK_LEVEL_HIGH,
+		summarizerv1.RiskLevel_RISK_LEVEL_CRITICAL,
+	} {
+		t.Run(level.String(), func(t *testing.T) {
+			c := makeClassifierWithActions(summarizerv1.ClassifierActions_builder{
+				EmitAuditEvent: summarizerv1.ClassifierActionMode_CLASSIFIER_ACTION_MODE_ENABLED,
+				FlagForReview:  summarizerv1.ClassifierActionMode_CLASSIFIER_ACTION_MODE_DISABLED,
+				RiskLevelFloor: level,
+			}.Build())
+
+			data, err := classifierResource{classifier: c}.MarshalJSON()
+			require.NoError(t, err)
+
+			back, err := classifierActionsFromFriendly(data)
+			require.NoError(t, err)
+			got, err := services.UnmarshalProtoResource[*summarizerv1.Classifier](back, services.DisallowUnknown())
+			require.NoError(t, err)
+
+			gotActions := got.GetSpec().GetActions()
+			require.Equal(t, summarizerv1.ClassifierActionMode_CLASSIFIER_ACTION_MODE_ENABLED, gotActions.GetEmitAuditEvent())
+			require.Equal(t, summarizerv1.ClassifierActionMode_CLASSIFIER_ACTION_MODE_DISABLED, gotActions.GetFlagForReview())
+			require.Equal(t, level, gotActions.GetRiskLevelFloor())
+		})
+	}
+}
+
+// Raw enum names, integers, and unknown values are rejected on input.
+func TestClassifierActionsFromFriendlyRejectsInvalid(t *testing.T) {
+	for _, tt := range []struct {
+		name    string
+		actions string
+	}{
+		{"toggle as enum string", `{"emit_audit_event":"CLASSIFIER_ACTION_MODE_ENABLED"}`},
+		{"toggle as integer", `{"flag_for_review":1}`},
+		{"risk level as enum string", `{"risk_level_floor":"RISK_LEVEL_HIGH"}`},
+		{"risk level as integer", `{"risk_level_floor":3}`},
+		{"risk level unknown", `{"risk_level_floor":"severe"}`},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			in := []byte(`{"kind":"classifier","version":"v1","metadata":{"name":"t"},` +
+				`"spec":{"kinds":["ssh"],"criteria":"c","actions":` + tt.actions + `}}`)
+			_, err := classifierActionsFromFriendly(in)
+			require.Error(t, err)
+		})
+	}
+}
+
+func unmarshalActions(t *testing.T, data []byte) map[string]any {
+	t.Helper()
+	var doc map[string]any
+	require.NoError(t, json.Unmarshal(data, &doc))
+	spec, ok := doc["spec"].(map[string]any)
+	require.True(t, ok, "spec missing from marshaled classifier")
+	actions, ok := spec["actions"].(map[string]any)
+	require.True(t, ok, "spec.actions missing from marshaled classifier")
+	return actions
+}