Skip to content

Dashboard#134

Open
khandrew-redhat wants to merge 5 commits intokrkn-chaos:mainfrom
khandrew-redhat:dashboard
Open

Dashboard#134
khandrew-redhat wants to merge 5 commits intokrkn-chaos:mainfrom
khandrew-redhat:dashboard

Conversation

@khandrew-redhat
Copy link
Copy Markdown

Description

Implements Kraken Dashboard command.
Typing either...
krknctl dashboard --kubeconfig <path/to/kubeconfig>
or
./krknctl dashboard --kubeconfig <path/to/kubeconfig>
...should spinup a podman container of the dashboard on the host machine. Note that the --kubeconfig flag is required.

KRKN_DASHBOARD_KUBECONFIG_BIND_SRC is set so podman-remote run -v gets the kubeconfig’s path on the Podman host, not the in-container path the app uses for uploads and fs checks. The volume mounts do not work without this implementation.
Because of this, krkn-chaos/krkn-dashboard#63 must be merged before this PR so the krknctl will pull the dashboard image with the latest changes.

Adds functionality to open up necessary ports for user/server interaction (3000, 8000), and for passing in required volume mounts.

Documentation

Documentation for this change will be handled by https://redhat.atlassian.net/browse/CHAOS-1467

image

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Add dashboard command with container port publishing support

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Implements new dashboard command to run krkn-dashboard web UI in container
• Adds port publishing support (3000, 8000) for dashboard and container access
• Extends Run and RunAttached methods with optional publish ports and Podman create options
• Adds dashboard image configuration to config.json with name and tag fields
• Implements Podman CLI-based container creation for macOS with platform and security options
Diagram
flowchart LR
  A["User runs<br/>dashboard command"] --> B["Prepare kubeconfig<br/>and data directory"]
  B --> C["Resolve container<br/>runtime socket"]
  C --> D["Configure volumes<br/>and environment"]
  D --> E["Pull dashboard image"]
  E --> F["Create and run<br/>container with ports"]
  F --> G["Dashboard available<br/>at localhost:3000"]
Loading

Grey Divider

File Changes

1. cmd/dashboard.go ✨ Enhancement +212/-0

New dashboard command implementation with container setup

cmd/dashboard.go


2. cmd/dashboard_podman.go ✨ Enhancement +60/-0

Podman-specific utilities for socket GID and machine detection

cmd/dashboard_podman.go


3. cmd/dashboard_test.go 🧪 Tests +55/-0

Unit tests for dashboard command and socket path parsing

cmd/dashboard_test.go


View more (10)
4. cmd/root.go ✨ Enhancement +3/-0

Register new dashboard command with root CLI

cmd/root.go


5. cmd/run.go ✨ Enhancement +2/-2

Update RunAttached and Run calls with new optional parameters

cmd/run.go


6. cmd/visualize.go ✨ Enhancement +1/-1

Update RunAttached call with new optional parameters

cmd/visualize.go


7. pkg/config/config.go ⚙️ Configuration changes +10/-0

Add dashboard image name and tag configuration fields

pkg/config/config.go


8. pkg/config/config.json ⚙️ Configuration changes +4/-2

Add dashboard image name and tag configuration values

pkg/config/config.json


9. pkg/scenarioorchestrator/common_functions.go ✨ Enhancement +3/-3

Update CommonRunAttached signature with publish ports and Podman options

pkg/scenarioorchestrator/common_functions.go


10. pkg/scenarioorchestrator/scenario_orchestrator.go ✨ Enhancement +11/-0

Add PodmanCreateOptions struct and update interface signatures

pkg/scenarioorchestrator/scenario_orchestrator.go


11. pkg/scenarioorchestrator/docker/scenario_orchestrator.go ✨ Enhancement +35/-6

Implement port publishing for Docker with conditional network mode

pkg/scenarioorchestrator/docker/scenario_orchestrator.go


12. pkg/scenarioorchestrator/podman/scenario_orchestrator.go ✨ Enhancement +82/-3

Add CLI-based Podman create for macOS with platform and security options

pkg/scenarioorchestrator/podman/scenario_orchestrator.go


13. pkg/scenarioorchestrator/scenarioorchestratortest/common_test_functions.go 🧪 Tests +10/-10

Update all test calls with new optional parameters

pkg/scenarioorchestrator/scenarioorchestratortest/common_test_functions.go


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Apr 10, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Dashboard ports exposed publicly🐞 Bug ⛨ Security
Description
Dashboard port publishing binds to all host interfaces by default, which can expose the UI/API to
the network. This is explicit in the Docker implementation (HostIP: 0.0.0.0) and Podman -p
defaults commonly publish on all interfaces.
Code

pkg/scenarioorchestrator/docker/scenario_orchestrator.go[R86-103]

+	if len(publishPorts) > 0 {
+		exposed := nat.PortSet{}
+		bindings := nat.PortMap{}
+		for _, pub := range publishPorts {
+			parts := strings.Split(pub, ":")
+			if len(parts) != 2 {
+				continue
+			}
+			hostPort, containerPort := parts[0], parts[1]
+			port, err := nat.NewPort("tcp", containerPort)
+			if err != nil {
+				return nil, fmt.Errorf("invalid publish port %q: %w", pub, err)
+			}
+			exposed[port] = struct{}{}
+			bindings[port] = []nat.PortBinding{{HostIP: "0.0.0.0", HostPort: hostPort}}
+		}
+		containerCfg.ExposedPorts = exposed
+		hostCfg.PortBindings = bindings
Evidence
The dashboard command always requests publishing 3000:3000 and 8000:8000. For Docker, when
publishPorts is set, the orchestrator binds HostIP to 0.0.0.0, exposing services on all
interfaces. For Podman on Darwin, the CLI path uses -p with the same strings (no loopback host IP
specified).

cmd/dashboard.go[24-29]
cmd/dashboard.go[166-173]
pkg/scenarioorchestrator/docker/scenario_orchestrator.go[79-106]
pkg/scenarioorchestrator/podman/scenario_orchestrator.go[58-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The dashboard publishes ports in a way that exposes them on all interfaces (0.0.0.0). This can unintentionally make the dashboard reachable from other machines on the network.
### Issue Context
- `cmd/dashboard.go` always publishes 3000 and 8000.
- Docker implementation explicitly uses `HostIP: "0.0.0.0"`.
- Podman CLI path uses `-p` without specifying loopback.
### Fix Focus Areas
- cmd/dashboard.go[24-29]
- cmd/dashboard.go[166-173]
- pkg/scenarioorchestrator/docker/scenario_orchestrator.go[86-104]
- pkg/scenarioorchestrator/podman/scenario_orchestrator.go[72-78]
### Suggested change
- Default to loopback-only publishing, e.g. support `127.0.0.1:3000:3000` and `127.0.0.1:8000:8000`.
- Update Docker port parsing to accept optional host IP (3-part form) and set `HostIP` accordingly; if only 2 parts are provided, default to `127.0.0.1`.
- For Podman CLI create, pass through the same 3-part `-p` strings so loopback binding is honored.
- (Optional) add a flag to allow users to opt into binding on 0.0.0.0 explicitly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Kubeconfig file left behind🐞 Bug ⛨ Security
Description
NewDashboardCommand copies the flattened kubeconfig into a temp dir but never deletes the
flattened kubeconfig file created by PrepareKubeconfig. This can leave sensitive credentials on
disk in the working directory after the dashboard stops.
Code

cmd/dashboard.go[R58-75]

+			preparedKubeconfig, err := utils.PrepareKubeconfig(foundKubeconfig, cfg)
+			if err != nil {
+				return err
+			}
+			if preparedKubeconfig == nil {
+				return fmt.Errorf("kubeconfig not found, please specify a valid kubeconfig path with --kubeconfig")
+			}
+
+			assetsDir, err := os.MkdirTemp("", "krknctl-dashboard-assets-*")
+			if err != nil {
+				return err
+			}
+			defer func() { _ = os.RemoveAll(assetsDir) }()
+
+			kubeconfigDest := filepath.Join(assetsDir, "kubeconfig")
+			if err := copyFile(*preparedKubeconfig, kubeconfigDest); err != nil {
+				return err
+			}
Evidence
NewDashboardCommand calls PrepareKubeconfig and then copies the resulting file into a temp
directory, but does not remove the original returned path. PrepareKubeconfig writes a new
flattened kubeconfig file into the current working directory with mode 0644, so the unremoved file
is both persistent and broadly readable.

cmd/dashboard.go[54-80]
pkg/scenarioorchestrator/utils/utils.go[30-52]
pkg/scenarioorchestrator/utils/utils.go[98-113]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`PrepareKubeconfig()` writes a flattened kubeconfig file into the current working directory and returns its path. `cmd/dashboard.go` then copies that file into a temp assets directory for mounting, but never deletes the original flattened file, leaving sensitive credentials behind on disk.
### Issue Context
This impacts every `krknctl dashboard` run and is especially risky because the flattened kubeconfig is written with permissive file mode in `PrepareKubeconfig()`.
### Fix Focus Areas
- cmd/dashboard.go[54-80]
- pkg/scenarioorchestrator/utils/utils.go[98-113]
### Suggested change
- In `cmd/dashboard.go`, once `preparedKubeconfig` is non-nil, add cleanup for the returned file (e.g., `defer os.Remove(*preparedKubeconfig)`), and ensure it runs even if later steps fail.
- (Optional but stronger) change `PrepareKubeconfig` to write with `0600` and/or to a temp directory instead of `os.Getwd()`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Kubeconfig requirement inconsistent🐞 Bug ≡ Correctness
Description
The dashboard command allows an empty --kubeconfig and will fall back to the default kubeconfig
location, so --kubeconfig is not actually required. This can run the dashboard against an
unintended default kubeconfig if multiple kubeconfigs are in use.
Code

cmd/dashboard.go[R54-64]

+			var foundKubeconfig *string
+			if kubeconfigPath != "" {
+				foundKubeconfig = &kubeconfigPath
+			}
+			preparedKubeconfig, err := utils.PrepareKubeconfig(foundKubeconfig, cfg)
+			if err != nil {
+				return err
+			}
+			if preparedKubeconfig == nil {
+				return fmt.Errorf("kubeconfig not found, please specify a valid kubeconfig path with --kubeconfig")
+			}
Evidence
If the flag is empty, foundKubeconfig remains nil and is passed to PrepareKubeconfig.
PrepareKubeconfig explicitly uses the default kubeconfig filename when the input is nil/empty,
enabling implicit kubeconfig selection.

cmd/dashboard.go[48-64]
pkg/scenarioorchestrator/utils/utils.go[30-41]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`cmd/dashboard.go` permits omitting `--kubeconfig` and falls back to the default kubeconfig path. If the intent is to require explicit kubeconfig selection for dashboard runs, this behavior is inconsistent.
### Issue Context
Current behavior:
- empty `--kubeconfig` -> `PrepareKubeconfig(nil, ...)` -> default kubeconfig path.
### Fix Focus Areas
- cmd/dashboard.go[48-64]
- cmd/dashboard.go[181-183]
- pkg/scenarioorchestrator/utils/utils.go[30-41]
### Suggested change
Choose one:
- Enforce requirement: return an error when `--kubeconfig` is empty and/or mark the flag required via Cobra (and update help text accordingly).
- Or explicitly document the fallback in the command description/help so users understand default kubeconfig will be used.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Publish ports silently ignored🐞 Bug ≡ Correctness
Description
Docker port publishing silently skips malformed publishPorts entries and still disables host
networking whenever publishPorts is non-empty. If all entries are malformed, the container starts
on the default network with no published ports, making it unreachable without a clear error.
Code

pkg/scenarioorchestrator/docker/scenario_orchestrator.go[R89-106]

+		for _, pub := range publishPorts {
+			parts := strings.Split(pub, ":")
+			if len(parts) != 2 {
+				continue
+			}
+			hostPort, containerPort := parts[0], parts[1]
+			port, err := nat.NewPort("tcp", containerPort)
+			if err != nil {
+				return nil, fmt.Errorf("invalid publish port %q: %w", pub, err)
+			}
+			exposed[port] = struct{}{}
+			bindings[port] = []nat.PortBinding{{HostIP: "0.0.0.0", HostPort: hostPort}}
+		}
+		containerCfg.ExposedPorts = exposed
+		hostCfg.PortBindings = bindings
+	} else {
+		hostCfg.NetworkMode = "host"
+	}
Evidence
When publishPorts is provided, the Docker orchestrator loops over entries and continues on
invalid formats rather than erroring. In this branch, it does not set NetworkMode: host; that only
happens in the len(publishPorts)==0 case, so a fully-invalid list can result in no bindings and no
host networking.

pkg/scenarioorchestrator/docker/scenario_orchestrator.go[86-106]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Malformed publish-port strings are silently ignored, and host networking is disabled as soon as `publishPorts` is non-empty. If all entries are invalid, the container will have no port bindings and no host networking, making it unreachable.
### Issue Context
This is in the Docker orchestrator port-binding logic.
### Fix Focus Areas
- pkg/scenarioorchestrator/docker/scenario_orchestrator.go[86-106]
### Suggested change
- Validate each `publishPorts` entry; return an error if any entry is malformed.
- After parsing, if no valid bindings were produced, return an error (or fall back to `NetworkMode: host`, depending on desired semantics).
- Consider using `strings.SplitN(pub, ":", 3)` and explicitly supporting formats like `hostIP:hostPort:containerPort`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread cmd/dashboard.go Outdated
Comment thread pkg/scenarioorchestrator/docker/scenario_orchestrator.go
@paigerube14
Copy link
Copy Markdown
Contributor

@khandrew-redhat can you take a look at the failed tests?

Signed-off-by: Khephren Andrews <khandrew@redhat.com>

removing empty file

Signed-off-by: Khephren Andrews <khandrew@redhat.com>

agent prompt edits

Signed-off-by: Khephren Andrews <khandrew@redhat.com>

temp image remove

Signed-off-by: Khephren Andrews <khandrew@redhat.com>

container test fix

Signed-off-by: Khephren Andrews <khandrew@redhat.com>

logs removal

Signed-off-by: Khephren Andrews <khandrew@redhat.com>

Test Updates

Signed-off-by: Khephren Andrews <khandrew@redhat.com>

another empty file removal

Signed-off-by: Khephren Andrews <khandrew@redhat.com>

test file edits

Signed-off-by: Khephren Andrews <khandrew@redhat.com>

dashboard route to prod

Signed-off-by: Khephren Andrews <khandrew@redhat.com>
Signed-off-by: Khephren Andrews <khandrew@redhat.com>
@khandrew-redhat
Copy link
Copy Markdown
Author

khandrew-redhat commented Apr 14, 2026

Failed tests are fixed on this PR (squashed commit) and associated dashboard PR has been edited.
Could I get a second pair of eyes on the test changes? I am not very proficient in go and I want to make sure they don't break the integrity of the existing tests to pass.
I know that the dashboard has different requirements than the typical Kraken ctl command

Signed-off-by: Khephren Andrews <khandrew@redhat.com>
Comment thread pkg/provider/registryv2/scenario_provider_test.go Outdated
Signed-off-by: Khephren Andrews <khandrew@redhat.com>
Signed-off-by: Khephren Andrews <khandrew@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants