feat: credentials script should support both username and password (#…

…2870)

* feat: credentials script should support both username and password

* feat: address review comments

* feat: the credential script should support both username and password (#2875)

* feat: address review comments

---------

Co-authored-by: Chris Grindstaff <chris@gstaff.org>

docs/configure-harvest-basic.md

@@ -267,41 +267,72 @@ Pollers:
 
 ## Credentials Script
 
-You can fetch authentication information via an external script by using the `credentials_script` section in
-the `Pollers` section of your `harvest.yml` as shown in the [example below](#example). 
+The `credentials_script` feature allows you to fetch authentication information via an external script. This can be configured in the `Pollers` section of your `harvest.yml` file, as shown in the example below.
 
-At runtime, Harvest will invoke the script referenced in the `credentials_script` `path` section.
-Harvest will call the script with two arguments like so: `./script $addr $username`. 
+At runtime, Harvest will invoke the script specified in the `credentials_script` `path` section. Harvest will call the script with one or two arguments depending on how your poller is configured in the `harvest.yml` file. The script will be called like this: `./script $addr` or `./script $addr $username`.
 
-- The first argument is the address of the cluster taken from your `harvest.yaml` file, section `Pollers addr`
-- The second argument is the username of the cluster taken from your `harvest.yaml` file, section `Pollers username`
+- The first argument `$addr` is the address of the cluster taken from the `addr` field under the `Pollers` section of your `harvest.yml` file.
+- The second argument `$username` is the username for the cluster taken from the `username` field under the `Pollers` section of your `harvest.yml` file. If your `harvest.yml` does not include a username, nothing will be passed.
 
-The script should use the two arguments to look up and return the password via the script's `standard out`.
-If the script doesn't finish within the specified `timeout`, Harvest will kill the script and any spawned processes.
+The script should  communicate the credentials to Harvest by writing the response to its standard output (stdout). Harvest supports two output formats from the script:
 
-Credential scripts are defined in your `harvest.yml` under the `Pollers` `credentials_script` section. 
-Below are the options for the `credentials_script` section  
+1. **YAML format:** If the script outputs a YAML object with `username` and `password` keys, Harvest will use both the `username` and `password` from the output. For example, if the script writes the following, Harvest will use `myuser` and `mypassword` for the poller's credentials.
+   ```yaml
+   username: myuser
+   password: mypassword
+   ```
+   If only the `password` is provided, Harvest will use the `username` from the `harvest.yml` file, if available. If your username or password contains spaces, `#`, or other characters with special meaning in YAML, make sure you quote the value like so:
+   `password: "my password with spaces"`
 
-| parameter | type                    | description                                                                                                                                                                    | default |
-|-----------|-------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------|
-| path      | string                  | absolute path to script that takes two arguments: addr and username, in that order                                                                                             |         |
-| schedule  | go duration or `always` | schedule used to call the authentication script. If the value is `always`, the script will be called everytime a password is requested, otherwise use the earlier cached value | 24h     |
-| timeout   | go duration             | amount of time Harvest will wait for the script to finish before killing it and descendents                                                                                    | 10s     |
+2. **Plain text format:** If the script outputs plain text, Harvest will use the output as the password. The `username` will be taken from the `harvest.yml` file, if available.  For example, if the script writes the following to its stdout, Harvest will use the username defined in that poller's section of the `harvest.yml` and `mypassword` for the poller's credentials.
+   ```
+   mypassword
+   ```
+
+If the script doesn't finish within the specified `timeout`, Harvest will terminate the script and any spawned processes.
+
+Credential scripts are defined under the `credentials_script` section within `Pollers` in your `harvest.yml`. Below are the options for the `credentials_script` section:
+
+| parameter | type                    | description                                                                                                                                                                  | default |
+|-----------|-------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------|
+| path      | string                  | Absolute path to the script that takes two arguments: `addr` and `username`, in that order.                                                                                  |         |
+| schedule  | go duration or `always` | Schedule for calling the authentication script. If set to `always`, the script is called every time a password is requested; otherwise, the previously cached value is used. | 24h     |
+| timeout   | go duration             | Maximum time Harvest will wait for the script to finish before terminating it and its descendants.                                                                           | 10s     |
 
 ### Example
 
+Here is an example of how to configure the `credentials_script` in the `harvest.yml` file:
+
 ```yaml
 Pollers:
-    ontap1:
-        datacenter: rtp
-        addr: 10.1.1.1
-        collectors:
-            - Rest
-            - RestPerf
-        credentials_script:
-            path: ./get_pass
-            schedule: 3h
-            timeout: 10s
+  ontap1:
+    datacenter: rtp
+    addr: 10.1.1.1
+    username: admin # Optional: if not provided, the script must return the username
+    collectors:
+      - Rest
+      - RestPerf
+    credentials_script:
+      path: ./get_credentials
+      schedule: 3h
+      timeout: 10s
+```
+
+In this example, the `get_credentials` script should be located in the same directory as the `harvest.yml` file and should be executable. It should output the credentials in either YAML or plain text format. Here are two example scripts:
+
+`get_credentials` that outputs YAML:
+```bash
+#!/bin/bash
+cat << EOF
+username: myuser
+password: mypassword
+EOF
+```
+
+`get_credentials` that outputs only the password in plain text:
+```bash
+#!/bin/bash
+echo "mypassword"
 ```
 
 ### Troubleshooting

pkg/auth/auth.go

@@ -13,6 +13,7 @@ import (
 	"github.com/netapp/harvest/v2/pkg/errs"
 	"github.com/netapp/harvest/v2/pkg/logging"
 	"github.com/netapp/harvest/v2/third_party/mergo"
+	"gopkg.in/yaml.v3"
 	"net/http"
 	"os"
 	"os/exec"
@@ -43,7 +44,7 @@ type Credentials struct {
 	nextUpdate     time.Time
 	logger         *logging.Logger
 	authMu         *sync.Mutex
-	cachedPassword string
+	cachedResponse ScriptResponse
 }
 
 // Expire will reset the credential schedule if the receiver has a CredentialsScript
@@ -71,39 +72,66 @@ func (c *Credentials) certs(poller *conf.Poller) (string, error) {
 	return c.fetchCerts(poller)
 }
 
-func (c *Credentials) password(poller *conf.Poller) (string, error) {
+func (c *Credentials) password(poller *conf.Poller) (ScriptResponse, error) {
 	if poller.CredentialsScript.Path == "" {
-		return poller.Password, nil
+		return ScriptResponse{
+			Data:     poller.Password,
+			Username: poller.Username,
+		}, nil
 	}
+
+	var response ScriptResponse
+	var err error
 	c.authMu.Lock()
 	defer c.authMu.Unlock()
 	if time.Now().After(c.nextUpdate) {
-		var err error
-		c.cachedPassword, err = c.fetchPassword(poller)
+		response, err = c.fetchPassword(poller)
 		if err != nil {
-			return "", err
+			return ScriptResponse{}, err
 		}
+		// Cache the new response and update the next update time.
+		c.cachedResponse = response
 		c.setNextUpdate()
 	}
-	return c.cachedPassword, nil
+	return c.cachedResponse, nil
 }
 
-func (c *Credentials) fetchPassword(p *conf.Poller) (string, error) {
-	return c.execScript(p.CredentialsScript.Path, "credential", p.CredentialsScript.Timeout, func(ctx context.Context, path string) *exec.Cmd {
+func (c *Credentials) fetchPassword(p *conf.Poller) (ScriptResponse, error) {
+	response, err := c.execScript(p.CredentialsScript.Path, "credential", p.CredentialsScript.Timeout, func(ctx context.Context, path string) *exec.Cmd {
 		return exec.CommandContext(ctx, path, p.Addr, p.Username) // #nosec
 	})
+	if err != nil {
+		return ScriptResponse{}, err
+	}
+	// If username is empty, use harvest config poller username
+	if response.Username == "" {
+		response.Username = p.Username
+	}
+	return response, nil
 }
 
 func (c *Credentials) fetchCerts(p *conf.Poller) (string, error) {
-	return c.execScript(p.CertificateScript.Path, "certificate", p.CertificateScript.Timeout, func(ctx context.Context, path string) *exec.Cmd {
+	response, err := c.execScript(p.CertificateScript.Path, "certificate", p.CertificateScript.Timeout, func(ctx context.Context, path string) *exec.Cmd {
 		return exec.CommandContext(ctx, path, p.Addr) // #nosec
 	})
+	if err != nil {
+		return "", err
+	}
+
+	// The script is expected to return only the certificate data, so we don't need to check for a username.
+	return response.Data, nil
 }
 
-func (c *Credentials) execScript(cmdPath string, kind string, timeout string, e func(ctx context.Context, path string) *exec.Cmd) (string, error) {
+type ScriptResponse struct {
+	Username string `yaml:"username"`
+	Data     string `yaml:"password"`
+}
+
+func (c *Credentials) execScript(cmdPath string, kind string, timeout string, e func(ctx context.Context, path string) *exec.Cmd) (ScriptResponse, error) {
+	response := ScriptResponse{}
 	lookPath, err := exec.LookPath(cmdPath)
 	if err != nil {
-		return "", fmt.Errorf("script lookup failed kind=%s err=%w", kind, err)
+		return response, fmt.Errorf("script lookup failed kind=%s err=%w", kind, err)
 	}
 	if timeout == "" {
 		timeout = defaultTimeout
@@ -141,7 +169,7 @@ func (c *Credentials) execScript(cmdPath string, kind string, timeout string, e
 			Str("stdout", stdout.String()).
 			Str("kind", kind).
 			Msg("Failed to start script")
-		return "", fmt.Errorf("script start failed script=%s kind=%s err=%w", lookPath, kind, err)
+		return response, fmt.Errorf("script start failed script=%s kind=%s err=%w", lookPath, kind, err)
 	}
 	err = cmd.Wait()
 	if err != nil {
@@ -152,9 +180,31 @@ func (c *Credentials) execScript(cmdPath string, kind string, timeout string, e
 			Str("stdout", stdout.String()).
 			Str("kind", kind).
 			Msg("Failed to execute script")
-		return "", fmt.Errorf("script execute failed script=%s kind=%s err=%w", lookPath, kind, err)
+		return response, fmt.Errorf("script execute failed script=%s kind=%s err=%w", lookPath, kind, err)
 	}
-	return strings.TrimSpace(stdout.String()), nil
+
+	err = yaml.Unmarshal(stdout.Bytes(), &response)
+	if err != nil {
+		// Log the error but do not return it, we will try to use the output as plain text next.
+		c.logger.Debug().Err(err).
+			Str("script", lookPath).
+			Str("timeout", duration.String()).
+			Str("stderr", stderr.String()).
+			Str("stdout", stdout.String()).
+			Str("kind", kind).
+			Msg("Failed to parse YAML output. Treating as plain text.")
+	}
+
+	if err == nil && response.Data != "" {
+		// If parsing is successful and data is not empty, return the response.
+		// Username is optional, so it's okay if it's not present.
+		return response, nil
+	}
+
+	// If YAML parsing fails or the data is empty,
+	// assume the output is the data (password or certificate) in plain text for backward compatibility.
+	response.Data = strings.TrimSpace(stdout.String())
+	return response, nil
 }
 
 func (c *Credentials) setNextUpdate() {
@@ -275,13 +325,13 @@ func getPollerAuth(c *Credentials, poller *conf.Poller) (PollerAuth, error) {
 		}, nil
 	}
 	if poller.CredentialsScript.Path != "" {
-		pass, err := c.password(poller)
+		response, err := c.password(poller)
 		if err != nil {
 			return PollerAuth{}, err
 		}
 		return PollerAuth{
-			Username:            poller.Username,
-			Password:            pass,
+			Username:            response.Username,
+			Password:            response.Data,
 			HasCredentialScript: true,
 			Schedule:            poller.CredentialsScript.Schedule,
 			insecureTLS:         insecureTLS,

pkg/auth/auth_test.go

@@ -438,6 +438,108 @@ Pollers:
     password: pass
     ca_cert: testdata/ca.pem`,
 		},
+		{
+			name:       "credentials_script returns username and password in YAML",
+			pollerName: "test",
+			want: PollerAuth{
+				Username:            "script-username",
+				Password:            "script-password",
+				HasCredentialScript: true,
+			},
+			yaml: `
+Pollers:
+  test:
+    addr: a.b.c
+    credentials_script:
+      path: testdata/get_credentials_yaml
+`,
+		},
+
+		{
+			name:       "credentials_script returns only password in plain text",
+			pollerName: "test",
+			want: PollerAuth{
+				Username:            "username", // Fallback to the username provided in the poller configuration
+				Password:            "plain-text-password",
+				HasCredentialScript: true,
+			},
+			yaml: `
+Pollers:
+  test:
+    addr: a.b.c
+    username: username
+    credentials_script:
+      path: testdata/get_password_plain
+`,
+		},
+		{
+			name:       "credentials_script returns only password in YAML format",
+			pollerName: "test",
+			want: PollerAuth{
+				Username:            "username", // Fallback to the username provided in the poller configuration
+				Password:            "password #\"`!@#$%^&*()-=[]|:'<>/ password",
+				HasCredentialScript: true,
+			},
+			yaml: `
+Pollers:
+  test:
+    addr: a.b.c
+    username: username
+    credentials_script:
+      path: testdata/get_credentials_yaml_password
+`,
+		},
+		{
+			name:       "credentials_script returns username and password in YAML, no username in poller config",
+			pollerName: "test",
+			want: PollerAuth{
+				Username:            "script-username",
+				Password:            "script-password",
+				HasCredentialScript: true,
+			},
+			yaml: `
+Pollers:
+  test:
+    addr: a.b.c
+    credentials_script:
+      path: testdata/get_credentials_yaml
+`,
+		},
+
+		{
+			name:       "credentials_script returns only password in plain text, no username in poller config",
+			pollerName: "test",
+			want: PollerAuth{
+				Username:            "", // No username provided, so it should be empty
+				Password:            "plain-text-password",
+				HasCredentialScript: true,
+			},
+			yaml: `
+Pollers:
+  test:
+    addr: a.b.c
+    credentials_script:
+      path: testdata/get_password_plain
+`,
+		},
+
+		{
+			name:       "credentials_script returns username and password in YAML via Heredoc",
+			pollerName: "test",
+			want: PollerAuth{
+				Username:            "myuser",
+				Password:            "my # password",
+				HasCredentialScript: true,
+			},
+			yaml: `
+Pollers:
+  test:
+    addr: a.b.c
+    username: username
+    credentials_script:
+      path: testdata/get_credentials_yaml_heredoc
+`,
+		},
 	}
 
 	hostname, err := os.Hostname()

pkg/auth/testdata/get_credentials_yaml

@@ -0,0 +1,4 @@
+#!/bin/bash
+# Used by pkg/auth/auth_test.go
+echo 'username: script-username'
+echo 'password: script-password'

pkg/auth/testdata/get_credentials_yaml_heredoc

@@ -0,0 +1,6 @@
+#!/bin/bash
+# Used by pkg/auth/auth_test.go
+cat << EOF
+username: myuser
+password: "my # password"
+EOF

pkg/auth/testdata/get_credentials_yaml_password

@@ -0,0 +1,6 @@
+#!/bin/bash
+# Used by pkg/auth/auth_test.go
+# Single quotes are used to avoid escaping special characters
+# Single quotes can not contain single quotes, so we use '\'' to close
+# the single quote, add a single quote, and then open the single quote again
+echo 'password: "password #\"`!@#$%^&*()-=[]|:'\''<>/ password"'

pkg/auth/testdata/get_password_plain

@@ -0,0 +1,3 @@
+#!/bin/bash
+# Used by pkg/auth/auth_test.go
+echo 'plain-text-password'