[cli] UI URL hints for common CLI commands #24454

philrenaud · 2024-11-13T15:46:09Z

Adds a "View more in the Web UI: $url" message at the end of common CLI commands to give users a way to dive deeper with a single click (or copy/paste)

Modify the ui block of your agent config with the following to disable: cli_url_links = false

Add a -ui flag to several common commands (nomad job status for example) for your default browser to open to the suggested page

…tput flag

aimeeu

a few style guide nits

aimeeu · 2024-12-17T15:00:13Z

website/content/docs/configuration/ui.mdx

@@ -58,6 +58,11 @@ and the configuration is individual to each agent.
 - `label` <code>([Label]: nil)</code> - Configures a user-defined
  label to display in the Nomad Web UI header.

+- `cli_url_links` `(bool: true)` - Controls whether CLI commands display hints
+  about equivalent UI pages. For example, when running `nomad server members`,
+  the CLI will show a message indicating where to find server information in


Suggested change

the CLI will show a message indicating where to find server information in

the CLI shows a message indicating where to find server information in

aimeeu · 2024-12-17T16:01:58Z

website/content/docs/configuration/ui.mdx

@@ -106,7 +111,7 @@ header sent with the web UI response.
 - `text` `(string: "")` - Specifies the text of the label that will be
  displayed in the header of the Web UI.
 - `background_color` `(string: "")` - The background color of the label to
-  be displayed. The Web UI will default to a black background. HEX values  
+  be displayed. The Web UI will default to a black background. HEX values


Suggested change

be displayed. The Web UI will default to a black background. HEX values

be displayed. The Web UI defaults to a black background. HEX values

present tense (I know this clashes with the copious amounts of future tense already in the docs. Eventually I will address that.)

jrasell

Thanks @philrenaud, I can see this being really useful.

I've added some inline comments after a first pass and will do some local testing as a followup. I have some slight trepidation that when using these commands an additional API call will be required to get the configuration value back, but I see its value and it's a fairly lightweight one.

Our CLI documentation includes the available flags for each command, so I think it would be nice to include this in the PR for completeness. Node status example.

jrasell · 2024-12-17T16:05:59Z

.changelog/24454.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+cli: Added UI URL hints to the end of common CLI commands and a -ui flag to auto-open them


Suggested change

cli: Added UI URL hints to the end of common CLI commands and a -ui flag to auto-open them

cli: Added UI URL hints to the end of common CLI commands and a `-ui` flag to auto-open them

jrasell · 2024-12-17T16:10:25Z

command/meta.go

+	return fmt.Sprintf("%s/ui%s", client.Address(), path), nil
+}
+
+func (m *Meta) showUIPath(ctx UIHintContext) (string, error) {


It looks like all callers ignore the error returned here. Do we expect this to be used in the future, should we check the error, or remove the return option from the function?

jrasell · 2024-12-17T16:14:42Z

nomad/structs/config/ui.go

 func DefaultUIConfig() *UIConfig {
+	enabled := true
 	return &UIConfig{
-		Enabled:               true,
+		Enabled:               enabled,
 		Consul:                &ConsulUIConfig{},
 		Vault:                 &VaultUIConfig{},
 		Label:                 &LabelUIConfig{},
 		ContentSecurityPolicy: DefaultCSPConfig(),
+		CLIURLLinks:           &enabled,
 	}
 }


We could remove the variable instantiation by using the pointer.Of function:

func DefaultUIConfig() *UIConfig { return &UIConfig{ Enabled: true, Consul: &ConsulUIConfig{}, Vault: &VaultUIConfig{}, Label: &LabelUIConfig{}, ContentSecurityPolicy: DefaultCSPConfig(), CLIURLLinks: pointer.Of(true), } }

schmichael · 2024-12-17T21:44:18Z

command/meta.go

+		path = strings.ReplaceAll(path, fmt.Sprintf(":%s", k), v)
+	}
+
+	return fmt.Sprintf("%s/ui%s", client.Address(), path), nil


I think this might lead to confusion where the CLI uses mTLS to talk to the Nomad HTTP API but the Web UI is only accessible through a proxy.

Yeah, that's a good point. I am actually worried that might sink this whole feature, the more I think about it.

Do we have any way of knowing at a config level if there is a reverse proxy set up? Or is that more extra-nomad than we can know about?

Do we have any way of knowing at a config level if there is a reverse proxy set up? Or is that more extra-nomad than we can know about?

Sadly I don't think we have anything today to do that, but a nomad.ui_url agent configuration parameter like consul and vault have would be sufficient. That would still require an Agent().Self() call to retrieve unless we did something very clever in the Agent HTTP API like sniff the User-Agent, and if it's the Nomad CLI: return an HTTP header with the ui_url. As hacky as that sounds I think returning extra metadata to the CLI might be a useful pattern?

schmichael · 2024-12-17T21:49:12Z

command/meta.go

+		return true
+	}
+
+	return !uiConfig["CLIURLLinks"].(bool)


This would cause new CLIs to panic when talking to old Agents that do not report this value: https://go.dev/play/p/Nm5dJx37eI0

schmichael · 2024-12-17T22:29:50Z

command/meta.go

+		return true
+	}
+	agentConfig := agent.Config
+	uiConfig, ok := agentConfig["UI"].(map[string]interface{})


I think this needs to be locally configurable instead of clusterwide configurable. For example it's probably desirable on operator machines and bastion hosts, but potentially undesirable on actual Nomad Clients where the CLI is only used for maintenance/debugging.

Since the UI is often proxied across many agents, if not the entire cluster, configuring this parameter clusterwide seems difficult.

I think we probably want to implement something like #24706 or at least an env var to let this behavior be user configurable.

We should probably be careful about doing implicit API calls from the CLI as well. Even though Agent().Self() should be cheap for the responding Agent, users running scripts over potentially slow links may see a significant slowdown from the extra roundtrips.

philrenaud self-assigned this Nov 13, 2024

philrenaud linked an issue Nov 13, 2024 that may be closed by this pull request

CLI: read-more with context #24445

Open

vercel bot deployed to Preview – nomad-ui November 13, 2024 16:10 View deployment

vercel bot deployed to Preview – nomad-ui November 13, 2024 21:55 View deployment

vercel bot deployed to Preview – nomad-ui November 14, 2024 15:30 View deployment

vercel bot deployed to Preview – nomad-ui November 14, 2024 20:50 View deployment

vercel bot deployed to Preview – nomad November 14, 2024 20:55 View deployment

vercel bot deployed to Preview – nomad-ui November 14, 2024 21:19 View deployment

philrenaud force-pushed the 24445-cli-read-more-with-context branch from 3621929 to 70dd502 Compare November 24, 2024 14:27

vercel bot deployed to Preview – nomad-ui November 24, 2024 14:28 View deployment

philrenaud force-pushed the 24445-cli-read-more-with-context branch from 70dd502 to f251fd3 Compare December 10, 2024 17:51

vercel bot deployed to Preview – nomad-ui December 10, 2024 17:52 View deployment

philrenaud added 8 commits December 12, 2024 16:58

Basic implementation for server members and node status

75d48a9

Commands for alloc status and job status

38ceefb

-ui flag for most commands

c48bff6

url hints for variables

25d4639

url hints for job dispatch, evals, and deployments

ca8e794

agent config ui.cli_url_links to disable

2df0494

Fix an issue where path prefix was presumed for variables

b77efe6

driver uncomment and general cleanup

67f5207

philrenaud force-pushed the 24445-cli-read-more-with-context branch from f251fd3 to 67f5207 Compare December 12, 2024 21:59

vercel bot deployed to Preview – nomad-ui December 12, 2024 22:01 View deployment

-ui flag on the generic status endpoint

6bbb350

vercel bot deployed to Preview – nomad-ui December 13, 2024 16:21 View deployment

Job run command gets namespaces, and no longer gets ui hints for --ou…

93d4d49

…tput flag

vercel bot deployed to Preview – nomad-ui December 16, 2024 15:22 View deployment

Dispatch command hints get a namespace, and bunch o tests

e93d487

vercel bot deployed to Preview – nomad-ui December 16, 2024 22:01 View deployment

Lots of tests depend on specific output, so let's not mess with them

bd21295

vercel bot deployed to Preview – nomad-ui December 17, 2024 02:08 View deployment

figured out what flagAddress is all about for testServer, oof

80b8ab2

vercel bot deployed to Preview – nomad-ui December 17, 2024 04:27 View deployment

Parallel outside of test instances

38a1b6b

vercel bot deployed to Preview – nomad-ui December 17, 2024 05:59 View deployment

Browser-opening test, sorta

67a6584

philrenaud force-pushed the 24445-cli-read-more-with-context branch from b5d296b to 67a6584 Compare December 17, 2024 06:05

philrenaud marked this pull request as ready for review December 17, 2024 06:05

philrenaud requested review from a team as code owners December 17, 2024 06:05

philrenaud requested review from heatlikeheatwave, tgross and jrasell December 17, 2024 06:05

vercel bot deployed to Preview – nomad-ui December 17, 2024 06:06 View deployment

aimeeu approved these changes Dec 17, 2024

View reviewed changes

jrasell reviewed Dec 17, 2024

View reviewed changes

schmichael mentioned this pull request Dec 17, 2024

CLI and API SDK Configuration Files #24706

Open

schmichael reviewed Dec 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cli] UI URL hints for common CLI commands #24454

[cli] UI URL hints for common CLI commands #24454

philrenaud commented Nov 13, 2024 •

edited

Loading

aimeeu left a comment

aimeeu Dec 17, 2024

aimeeu Dec 17, 2024

jrasell left a comment

jrasell Dec 17, 2024

jrasell Dec 17, 2024

jrasell Dec 17, 2024

schmichael Dec 17, 2024

philrenaud Dec 18, 2024

schmichael Dec 18, 2024 •

edited

Loading

schmichael Dec 17, 2024

schmichael Dec 17, 2024

	the CLI will show a message indicating where to find server information in
	the CLI shows a message indicating where to find server information in

	be displayed. The Web UI will default to a black background. HEX values
	be displayed. The Web UI defaults to a black background. HEX values

		@@ -0,0 +1,3 @@
		```release-note:improvement
		cli: Added UI URL hints to the end of common CLI commands and a -ui flag to auto-open them

[cli] UI URL hints for common CLI commands #24454

Are you sure you want to change the base?

[cli] UI URL hints for common CLI commands #24454

Conversation

philrenaud commented Nov 13, 2024 • edited Loading

aimeeu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrasell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schmichael Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philrenaud commented Nov 13, 2024 •

edited

Loading

schmichael Dec 18, 2024 •

edited

Loading