Improve ban subcommand by jrosental · Pull Request #123 · kubesaw/ksctl

jrosental · 2025-09-02T17:23:45Z

See https://issues.redhat.com/browse/SANDBOX-1402

… is invoked without a reason explicitly

MatousJobanek

Great start 👍
Let me know if you need any help with the unit tests - see my last comment of the review for the hints.

MatousJobanek · 2025-09-08T12:31:17Z


+const (
+	menuKey       string = "menu.json"
+	configMapName string = "banning-reasons"


I would probably name it ban-reason-config

MatousJobanek · 2025-09-08T12:36:50Z

+	if len(args) == 2 {
+		// Traditional usage: both usersignup name and ban reason provided
+		banReason = args[1]
+	} else {
+		// Interactive mode: only usersignup name provided, need to get reason from ConfigMap menu
+		ctx.Printlnf("No ban reason provided. Checking for available reasons from ConfigMap...")


I would rather use the following:

drop the second argument (the ban reason)

always try to get the ConfigMap

if ConfigMap is not available (or is empty) then ask user to provide the reason as an input in the interactive flow

MatousJobanek · 2025-09-08T12:37:58Z

+		if apierrors.IsNotFound(err) {
+			return nil, fmt.Errorf("failed to get ConfigMap: %w", err)
+		}
+		return nil, err


You can simplify it only to

Suggested change

if apierrors.IsNotFound(err) {

return nil, fmt.Errorf("failed to get ConfigMap: %w", err)

}

return nil, err

return nil, err

Ok, I just wanted to be more specific here.

MatousJobanek · 2025-09-08T12:40:24Z

+	configMap := &corev1.ConfigMap{}
+	err = cl.Get(context.TODO(), types.NamespacedName{
+		Name:      configMapName,
+		Namespace: "toolchain-host-operator",


Suggested change

Namespace: "toolchain-host-operator",

Namespace: cfg.OperatorNamespace,

so it's not hard-coded, but loaded from the ksctl.yaml file

MatousJobanek · 2025-09-08T12:43:05Z

+	if menuJSON, exists := configMap.Data[menuKey]; exists && menuJSON != "" {
+		//var menus []Menu
+		if err := json.Unmarshal([]byte(menuJSON), &menus); err != nil {
+			return nil, fmt.Errorf("ConfigMap doesn't contain %s key: %w", menuKey, err)


the error returned is not because the ConfigMap wouldn't contain the key, but because the value doesn't contain the correct JSON format.
The error that it doesn't contain the key should be returned somewhere else

MatousJobanek · 2025-09-08T13:00:01Z

+	t.Run("single workload menu item", func(t *testing.T) {
+		// This test demonstrates the structure but cannot test interactivity
+
+		// given
+		menu := []cmd.Menu{
+			{
+				Kind:        "workload",
+				Description: "Select workload type",
+				Options:     []string{"container", "vm"},
+			},
+		}
+
+		// Verify menu structure is correct for processing
+		assert.Len(t, menu, 1)
+		assert.Equal(t, "workload", menu[0].Kind)
+		assert.Equal(t, "Select workload type", menu[0].Description)
+		assert.Len(t, menu[0].Options, 2)
+		assert.Contains(t, menu[0].Options, "container")
+	})
+
+	t.Run("multiple menu items structure", func(t *testing.T) {
+		// given
+		menu := []cmd.Menu{
+			{
+				Kind:        "workload",
+				Description: "Select workload type",
+				Options:     []string{"container", "vm"},
+			},
+			{
+				Kind:        "behavior",
+				Description: "Select behavior classification",
+				Options:     []string{"malicious", "suspicious", "policy-violation"},
+			},
+			{
+				Kind:        "detection",
+				Description: "Select detection mechanism",
+				Options:     []string{"automated", "manual", "user-report"},
+			},
+		}
+
+		// Verify menu structure supports all BanInfo fields
+		kindMap := make(map[string]bool)
+		for _, item := range menu {
+			kindMap[item.Kind] = true
+			assert.NotEmpty(t, item.Description)
+			assert.NotEmpty(t, item.Options)
+		}
+
+		assert.True(t, kindMap["workload"])
+		assert.True(t, kindMap["behavior"])
+		assert.True(t, kindMap["detection"])
+	})


correct me if I missed something, but it looks like that you don't verify any actual logic in these two tests.

MatousJobanek · 2025-09-08T13:06:51Z

+func TestBanMenuMappingLogic(t *testing.T) {
+	// This test verifies the mapping logic that converts menu selections to BanInfo
+
+	t.Run("verify BanInfo field mapping", func(t *testing.T) {
+		// Test data that simulates what would be collected from the interactive menu
+		testCases := []struct {
+			name     string
+			kind     string
+			answer   string
+			expected func(*cmd.BanInfo) string
+		}{
+			{
+				name:   "workload mapping",
+				kind:   "workload",
+				answer: "compute-intensive",
+				expected: func(info *cmd.BanInfo) string {
+					return info.WorkloadType
+				},
+			},
+			{
+				name:   "behavior mapping",
+				kind:   "behavior",
+				answer: "malicious",
+				expected: func(info *cmd.BanInfo) string {
+					return info.BehaviorClassification
+				},
+			},
+			{
+				name:   "detection mapping",
+				kind:   "detection",
+				answer: "automated",
+				expected: func(info *cmd.BanInfo) string {
+					return info.DetectionMechanism
+				},
+			},
+		}
+
+		for _, tc := range testCases {
+			t.Run(tc.name, func(t *testing.T) {
+				// This demonstrates the expected mapping behavior
+				banInfo := &cmd.BanInfo{}
+
+				// Simulate the switch statement logic from banMenu
+				switch tc.kind {
+				case "workload":
+					banInfo.WorkloadType = tc.answer
+				case "behavior":
+					banInfo.BehaviorClassification = tc.answer
+				case "detection":
+					banInfo.DetectionMechanism = tc.answer
+				}
+
+				assert.Equal(t, tc.expected(banInfo), tc.answer)
+			})
+		}
+	})
+}
+
+func TestMenuStruct(t *testing.T) {
+	t.Run("JSON unmarshaling works correctly", func(t *testing.T) {
+		// given
+		jsonData := `[
+			{
+				"kind": "workload",
+				"description": "Select workload type",
+				"options": ["container", "vm"]
+			},
+			{
+				"kind": "behavior", 
+				"description": "Select behavior classification",
+				"options": ["malicious", "suspicious"]
+			}
+		]`
+
+		// when
+		var menus []cmd.Menu
+		err := json.Unmarshal([]byte(jsonData), &menus)
+
+		// then
+		require.NoError(t, err)
+		assert.Len(t, menus, 2)
+
+		// Verify first menu
+		assert.Equal(t, "workload", menus[0].Kind)
+		assert.Equal(t, "Select workload type", menus[0].Description)
+		assert.Len(t, menus[0].Options, 2)
+		assert.Contains(t, menus[0].Options, "container")
+		assert.Contains(t, menus[0].Options, "vm")
+
+		// Verify second menu
+		assert.Equal(t, "behavior", menus[1].Kind)
+		assert.Equal(t, "Select behavior classification", menus[1].Description)
+		assert.Len(t, menus[1].Options, 2)
+		assert.Contains(t, menus[1].Options, "malicious")
+		assert.Contains(t, menus[1].Options, "suspicious")
+	})
+
+	t.Run("empty JSON array unmarshals correctly", func(t *testing.T) {
+		// given
+		jsonData := `[]`
+
+		// when
+		var menus []cmd.Menu
+		err := json.Unmarshal([]byte(jsonData), &menus)
+
+		// then
+		require.NoError(t, err)
+		assert.Empty(t, menus)
+	})
+
+	t.Run("malformed JSON returns error", func(t *testing.T) {
+		// given
+		jsonData := `[{invalid json`
+
+		// when
+		var menus []cmd.Menu
+		err := json.Unmarshal([]byte(jsonData), &menus)
+
+		// then
+		require.Error(t, err)
+	})
+}
+
+func TestBanInfoStruct(t *testing.T) {
+	t.Run("JSON marshaling works correctly", func(t *testing.T) {
+		// given
+		banInfo := &cmd.BanInfo{
+			WorkloadType:           "container",
+			BehaviorClassification: "malicious",
+			DetectionMechanism:     "automated",
+		}
+
+		// when
+		jsonData, err := json.Marshal(banInfo)
+
+		// then
+		require.NoError(t, err)
+		assert.Contains(t, string(jsonData), "container")
+		assert.Contains(t, string(jsonData), "malicious")
+		assert.Contains(t, string(jsonData), "automated")
+
+		// Verify it can be unmarshaled back
+		var unmarshaled cmd.BanInfo
+		err = json.Unmarshal(jsonData, &unmarshaled)
+		require.NoError(t, err)
+		assert.Equal(t, banInfo.WorkloadType, unmarshaled.WorkloadType)
+		assert.Equal(t, banInfo.BehaviorClassification, unmarshaled.BehaviorClassification)
+		assert.Equal(t, banInfo.DetectionMechanism, unmarshaled.DetectionMechanism)
+	})
+
+	t.Run("empty BanInfo marshals to empty fields", func(t *testing.T) {
+		// given
+		banInfo := &cmd.BanInfo{}
+
+		// when
+		jsonData, err := json.Marshal(banInfo)
+
+		// then
+		require.NoError(t, err)
+		assert.Contains(t, string(jsonData), `"workloadType":""`)
+		assert.Contains(t, string(jsonData), `"behaviorClassification":""`)
+		assert.Contains(t, string(jsonData), `"detectionMechanism":""`)
+	})
+}
+
+// TestBanMenuInteractiveLogic tests the interactive menu logic in BanMenu function (lines 76-110)
+func TestBanMenuLogic(t *testing.T) {
+	t.Run("BanMenu with menu content creates proper data structures", func(t *testing.T) {
+		// This test verifies the logic of creating huh.Option structures and the mapping
+		// We can't test the actual interaction, but we can test the data preparation
+
+		// given
+		menu := []cmd.Menu{
+			{
+				Kind:        "workload",
+				Description: "Select workload type",
+				Options:     []string{"container", "vm"},
+			},
+			{
+				Kind:        "behavior",
+				Description: "Select behavior classification",
+				Options:     []string{"malicious", "suspicious"},
+			},
+		}
+
+		// Verify the menu structure that would be processed in lines 77-95
+		for _, item := range menu {
+			// Simulate the options creation logic from lines 79-82
+			options := make([]map[string]string, len(item.Options))
+			for i, opt := range item.Options {
+				options[i] = map[string]string{"Key": opt, "Value": opt}
+			}
+
+			// Verify options are created correctly
+			assert.Len(t, options, len(item.Options))
+			for i, opt := range item.Options {
+				assert.Equal(t, opt, options[i]["Key"])
+				assert.Equal(t, opt, options[i]["Value"])
+			}
+		}
+	})
+
+	t.Run("BanMenu mapping logic from answers to BanInfo", func(t *testing.T) {
+		// Test the switch statement logic that maps answers to BanInfo fields
+
+		testCases := []struct {
+			name         string
+			answers      map[string]string
+			expectedInfo *cmd.BanInfo
+		}{
+			{
+				name: "all fields mapped correctly",
+				answers: map[string]string{
+					"workload":  "container",
+					"behavior":  "malicious",
+					"detection": "automated",
+				},
+				expectedInfo: &cmd.BanInfo{
+					WorkloadType:           "container",
+					BehaviorClassification: "malicious",
+					DetectionMechanism:     "automated",
+				},
+			},
+			{
+				name: "partial mapping - only workload",
+				answers: map[string]string{
+					"workload": "vm",
+				},
+				expectedInfo: &cmd.BanInfo{
+					WorkloadType:           "vm",
+					BehaviorClassification: "",
+					DetectionMechanism:     "",
+				},
+			},
+			{
+				name: "unknown kind ignored",
+				answers: map[string]string{
+					"workload": "container",
+					"unknown":  "should-be-ignored",
+				},
+				expectedInfo: &cmd.BanInfo{
+					WorkloadType:           "container",
+					BehaviorClassification: "",
+					DetectionMechanism:     "",
+				},
+			},
+			{
+				name:    "empty answers",
+				answers: map[string]string{},
+				expectedInfo: &cmd.BanInfo{
+					WorkloadType:           "",
+					BehaviorClassification: "",
+					DetectionMechanism:     "",
+				},
+			},
+		}
+
+		for _, tc := range testCases {
+			t.Run(tc.name, func(t *testing.T) {
+				// Simulate the logic from lines 102-112
+				banInfo := &cmd.BanInfo{}
+
+				for kind, answer := range tc.answers {
+					switch kind {
+					case "workload":
+						banInfo.WorkloadType = answer
+					case "behavior":
+						banInfo.BehaviorClassification = answer
+					case "detection":
+						banInfo.DetectionMechanism = answer
+					}
+				}
+
+				assert.Equal(t, tc.expectedInfo.WorkloadType, banInfo.WorkloadType)
+				assert.Equal(t, tc.expectedInfo.BehaviorClassification, banInfo.BehaviorClassification)
+				assert.Equal(t, tc.expectedInfo.DetectionMechanism, banInfo.DetectionMechanism)
+			})
+		}
+	})
+}


the same for these test cases, I don't see any "band command" logic that would be tested here.
We don't need to test the logic of json.Marshal per se - we expect that it works.
Also, we should not simulate the actual logic from the command - we should rather call the logic to test that it really works.
In other words, unit-tests should always test the "production" logic.

MatousJobanek · 2025-09-08T13:11:27Z

+
+// TestBanInteractiveModeWithValidConfigMap tests the complete interactive flow
+func TestBanWithValidConfigMap(t *testing.T) {
+	t.Run("Ban function interactive mode output messages (line 167)", func(t *testing.T) {


this test name looks a bit weird.

MatousJobanek · 2025-09-08T13:35:57Z

+				options[i] = huh.Option[string]{Key: opt, Value: opt}
+			}
+
+			form := huh.NewSelect[string]().


according to the huh README, it looks like it should be defined as part of the Form & Group:
https://github.com/charmbracelet/huh?tab=readme-ov-file#tutorial

MatousJobanek · 2025-09-08T13:59:20Z

+		// This test will fail at the interactive part, but we can verify initial processing
+		// We expect it to get to the interactive menu and fail there
+
+		// when
+		err := cmd.Ban(ctx, userSignup.Name)
+
+		// then
+		// Should fail at the interactive part (huh.Select.Run()), but we can verify:


I see that you still have issues with testing the interactive mode. It's a bit tricky but there is a way to do that.
The problematic line is when it tries to "Run" the form:

form.Run()

it returns an error.
The solution would be moving this "run" call into a parameter of the Ban command - something like this:

type runFormFunc func(form *huh.Form) error func Ban(ctx *clicontext.CommandContext, runForm runFormFunc, args ...string) error {

so it can be used:

RunE: func(cmd *cobra.Command, args []string) error { term := ioutils.NewTerminal(cmd.InOrStdin, cmd.OutOrStdout) ctx := clicontext.NewCommandContext(term, client.DefaultNewClient) showForm := func(form *huh.Form) error { if err := form.Run(); err != nil { return fmt.Errorf("failed to show interactive menu: %w", err) } return nil } return Ban(ctx, showForm, args...) },

but in unit tests you can do something else. There are a bunch of other methods you can call on the form:

form.Init() // to init the form form.View() // to return the form as the string (for example to be printed out in case of debugging)

but what is more important, you can select between the options and fields via form.Update

form.Update(tea.KeyMsg{Type: tea.KeyDown}) // moves the selection one option down form.Update(huh.NextField()) // goes to the next selection field

not sure if there is a better way of testing the huh forms. @xcoulon do you know?

…e TestBanWithValidConfigMap function

…ason if the configmap doesn't exist or is empty

…ing interactive menu

MatousJobanek

There are some zombies 👻 (commented lines of code), can you please clean them up?

MatousJobanek · 2025-09-24T13:06:09Z

+	if len(args) == 0 {
+		return fmt.Errorf("UserSignup name is required")
+	}


this is actually not needed - the validation is already done by Cobra when you defined

Args: cobra.ExactArgs(1),

Suggested change

if len(args) == 0 {

return fmt.Errorf("UserSignup name is required")

}

MatousJobanek · 2025-09-24T13:11:33Z

+	cfgMapContent, err := getValuesFromConfigMap(ctx)
+
+	if err != nil || len(cfgMapContent) == 0 {
+		fmt.Printf("failed to load reasons from ConfigMap: %s", err)


Suggested change

fmt.Printf("failed to load reasons from ConfigMap: %s", err)

if err != nil {

ctx.Printlnf("failed to load reasons from ConfigMap %q: %s", configMapName, err)

} else {

ctx.Printlnf("the provided ConfigMap %q is empty", configMapName)

}

MatousJobanek · 2025-09-24T13:12:38Z

+		//err := runForm(form)
+		err := runForm(form)
+		if err != nil {
+			return fmt.Errorf("banning option could not be obtained: %w", err)


Suggested change

return fmt.Errorf("banning option could not be obtained: %w", err)

return fmt.Errorf("ban reason could not be obtained: %w", err)

MatousJobanek · 2025-09-24T13:22:16Z

+		fmt.Printf("\nYour selection:\n")
+		for kind, optionSelected := range answers {
+			fmt.Printf("- %s:\t%s\n", kind, optionSelected)


let's use the ctx *clicontext.CommandContext for printing out messages as we do it at other places

MatousJobanek · 2025-09-24T13:48:40Z

+func TestMenuStruct(t *testing.T) {
+	t.Run("JSON unmarshaling works correctly", func(t *testing.T) {
+		// given
+		jsonData := `[
+			{
+				"kind": "workload",
+				"description": "Select workload type",
+				"options": ["container", "vm"]
+			},
+			{
+				"kind": "behavior", 
+				"description": "Select behavior classification",
+				"options": ["malicious", "suspicious"]
+			}
+		]`
+
+		// when
+		var menus []cmd.Menu
+		err := json.Unmarshal([]byte(jsonData), &menus)
+
+		// then
+		require.NoError(t, err)
+		assert.Len(t, menus, 2)
+
+		// Verify first menu
+		assert.Equal(t, "workload", menus[0].Kind)
+		assert.Equal(t, "Select workload type", menus[0].Description)
+		assert.Len(t, menus[0].Options, 2)
+		assert.Contains(t, menus[0].Options, "container")
+		assert.Contains(t, menus[0].Options, "vm")
+
+		// Verify second menu
+		assert.Equal(t, "behavior", menus[1].Kind)
+		assert.Equal(t, "Select behavior classification", menus[1].Description)
+		assert.Len(t, menus[1].Options, 2)
+		assert.Contains(t, menus[1].Options, "malicious")
+		assert.Contains(t, menus[1].Options, "suspicious")
+	})
+
+	t.Run("empty JSON array unmarshals correctly", func(t *testing.T) {
+		// given
+		jsonData := `[]`
+
+		// when
+		var menus []cmd.Menu
+		err := json.Unmarshal([]byte(jsonData), &menus)
+
+		// then
+		require.NoError(t, err)
+		assert.Empty(t, menus)
+	})
+
+	t.Run("malformed JSON returns error", func(t *testing.T) {
+		// given
+		jsonData := `[{invalid json`
+
+		// when
+		var menus []cmd.Menu
+		err := json.Unmarshal([]byte(jsonData), &menus)
+
+		// then
+		require.Error(t, err)
+	})
+}


as I mentioned in my previous review - these tests don't verify anything from the "ban command" logic, let's drop them.

MatousJobanek · 2025-09-24T13:54:46Z

+		emptyConfigMap := &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "ban-reason-config",
+				Namespace: test.HostOperatorNs,
+			},
+			Data: map[string]string{
+				"menu.json": "[]", // Empty array
+			},
+		}


I think that the creation is used in the same way in the other tests, you could extract that to a helper function

func newBanReasonConfigMap(key, value string) *corev1.ConfigMap { return &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: "ban-reason-config", Namespace: test.HostOperatorNs, }, Data: map[string]string{ key: value, }, } }

and then use it:

emptyConfigMap := newBanReasonConfigMap("Menu.json", "[]") // Empty array

and

configMapWithoutMenu := newBanReasonConfigMap("other-key", "some-value") // No Menu.json key

etc...

MatousJobanek · 2025-09-24T13:59:36Z

+
+func TestBanCmdInteractiveMode(t *testing.T) {
+	t.Run("interactive mode with ConfigMap present", func(t *testing.T) {
+		t.Skip("Skipping interactive test - requires actual terminal interaction")


why skipping?

MatousJobanek · 2025-09-24T14:00:21Z

+
+		// then
+		require.NoError(t, err)
+		assert.Contains(t, term.Output(), "Checking for available reasons from ConfigMap...")


with my proposed changes in the other comment, you can check the actual reason why the select mode wasn't used

Suggested change

assert.Contains(t, term.Output(), "Checking for available reasons from ConfigMap...")

assert.Contains(t, term.Output(), "the provided ConfigMap \"ban-reason-config\" is empty")

MatousJobanek

Looks good. let's clean it up from all the zombies.
There is actually another thing to be updated - we need to add "read ConfigMap" permissions to the "ban-user" role here:

ksctl/resources/roles/host.yaml

Lines 111 to 134 in 66afa66

    
           - kind: Role 
        
             apiVersion: rbac.authorization.k8s.io/v1 
        
             metadata: 
        
               name: ban-user 
        
               labels: 
        
                 provider: ksctl 
        
             rules: 
        
             - apiGroups: 
        
               - toolchain.dev.openshift.com 
        
               resources: 
        
               - "bannedusers" 
        
               verbs: 
        
               - "get" 
        
               - "list" 
        
               - "create" 
        
               - "delete" 
        
             - apiGroups: 
        
               - toolchain.dev.openshift.com 
        
               resources: 
        
               - "usersignups" 
        
               - "masteruserrecords" 
        
               verbs: 
        
               - "get" 
        
               - "list"

This template is used to generate the permissions based on ksctl-admins.yaml file as we have it in sandbox-sre

MatousJobanek · 2025-10-01T07:17:49Z

+			)
+
+			if err := runForm(form); err != nil {
+				return nil, err //fmt.Errorf("failed to show interactive menu: %w", err)


let's clean up all these zombies 👻

Suggested change

return nil, err //fmt.Errorf("failed to show interactive menu: %w", err)

return nil, err

MatousJobanek · 2025-10-01T07:20:14Z

+
+	}
+
+	//return answers, nil


Suggested change

//return answers, nil

MatousJobanek · 2025-10-01T07:27:48Z

+
+	var menus []Menu
+	if menuJSON, exists := configMap.Data[menuKey]; exists && menuJSON != "" {
+		//var menus []Menu


Suggested change

//var menus []Menu

MatousJobanek · 2025-10-01T07:29:42Z

+
+		ctx.Printlnf("\nYour selection:\n")
+		for kind, optionSelected := range answers {
+			fmt.Printf("- %s:\t%s\n", kind, optionSelected)


Suggested change

fmt.Printf("- %s:\t%s\n", kind, optionSelected)

ctx.Printf("- %s:\t%s\n", kind, optionSelected)

MatousJobanek · 2025-10-01T07:32:24Z

+		/*		emptyConfigMap := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "ban-reason-config",
+					Namespace: test.HostOperatorNs,
+				},
+				Data: map[string]string{
+					"menu.json": "[]", // Empty array
+				},
+			}*/


🙃

Suggested change

/* emptyConfigMap := &corev1.ConfigMap{

ObjectMeta: metav1.ObjectMeta{

Name: "ban-reason-config",

Namespace: test.HostOperatorNs,

},

Data: map[string]string{

"menu.json": "[]", // Empty array

},

}*/

and in other test-cases too

MatousJobanek · 2025-10-01T07:33:49Z

+		err := cmd.Ban(ctx, func(form *huh.Form) error {
+			form.Init()
+			form.Update(banReasonInput)
+			//	form.View()


👻

Suggested change

// form.View()

MatousJobanek

Nice 🥇 Great job 🥇 🚀

codecov · 2025-10-07T13:38:11Z

Codecov Report

❌ Patch coverage is 79.13043% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.74%. Comparing base (66afa66) to head (afec370).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/cmd/ban.go	77.98%	18 Missing and 6 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #123      +/-   ##
==========================================
+ Coverage   70.54%   70.74%   +0.20%     
==========================================
  Files          56       56              
  Lines        4013     4116     +103     
==========================================
+ Hits         2831     2912      +81     
- Misses        958      974      +16     
- Partials      224      230       +6

Files with missing lines	Coverage Δ
pkg/test/banneduser.go	`100.00% <100.00%> (ø)`
pkg/cmd/ban.go	`75.90% <77.98%> (+3.17%)`	⬆️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jrosental and others added 11 commits August 24, 2025 20:23

Add getBanningReasonsFromConfigMap and showInteractiveBanMenu functions

bc79dec

add additional information gathered through menus when ban subcommand…

8287756

… is invoked without a reason explicitly

Revert go.mod and go.sum to Go 1.22

044271a

adding github.com/charmbracelet/huh@v0.6.0 as dependency for Go 1.22

f19a322

removing unused FormatBanReason method

a397d83

Add additional tests for banMenu() function

c680bf3

Add additional testing functions

81c52d4

fix typo

07969b5

Merge branch 'master' into improve-ban-subcommand

c9046aa

make changes suggested by testify

3ca5967

Merge branch 'master' into improve-ban-subcommand

3647536

MatousJobanek reviewed Sep 8, 2025

View reviewed changes

jrosental added 8 commits September 16, 2025 11:59

Change hardcoded values by constant names, wrong name in comment abov…

3a9aa38

…e TestBanWithValidConfigMap function

change to expect just the usersignup parameter and ask for the ban re…

694819a

…ason if the configmap doesn't exist or is empty

Addressing comments from kubesaw#123 (review)

dabdd80

fix uppercase string in error

301c62b

Modify Ban function to accept a function type parameter to allow test…

0e5135c

…ing interactive menu

Merge branch 'master' into improve-ban-subcommand

18d8e8b

fix conflicts in go.mod and go.sum

5db6ea1

Remove unused helper function createConfigMap

35457f0

MatousJobanek reviewed Sep 24, 2025

View reviewed changes

Addressing reviewer comments

dfbdc8a

MatousJobanek reviewed Oct 1, 2025

View reviewed changes

jrosental added 2 commits October 1, 2025 15:13

Cleaning comments

4171542

removing trailing spaces

afec370

MatousJobanek approved these changes Oct 6, 2025

View reviewed changes

jrosental merged commit 8c2a275 into kubesaw:master Oct 7, 2025
9 checks passed

	Namespace: "toolchain-host-operator",
	Namespace: cfg.OperatorNamespace,

	if len(args) == 0 {
	return fmt.Errorf("UserSignup name is required")
	}

	return fmt.Errorf("banning option could not be obtained: %w", err)
	return fmt.Errorf("ban reason could not be obtained: %w", err)

	assert.Contains(t, term.Output(), "Checking for available reasons from ConfigMap...")
	assert.Contains(t, term.Output(), "the provided ConfigMap \"ban-reason-config\" is empty")

	- kind: Role
	apiVersion: rbac.authorization.k8s.io/v1
	metadata:
	name: ban-user
	labels:
	provider: ksctl
	rules:
	- apiGroups:
	- toolchain.dev.openshift.com
	resources:
	- "bannedusers"
	verbs:
	- "get"
	- "list"
	- "create"
	- "delete"
	- apiGroups:
	- toolchain.dev.openshift.com
	resources:
	- "usersignups"
	- "masteruserrecords"
	verbs:
	- "get"
	- "list"

	return nil, err //fmt.Errorf("failed to show interactive menu: %w", err)
	return nil, err

	fmt.Printf("- %s:\t%s\n", kind, optionSelected)
	ctx.Printf("- %s:\t%s\n", kind, optionSelected)

Uh oh!

Conversation

jrosental commented Sep 2, 2025

Uh oh!

MatousJobanek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatousJobanek left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatousJobanek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatousJobanek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Oct 7, 2025

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

MatousJobanek left a comment •

edited

Loading