The Reluctant Software Developer: Contributing to HashiCorp Vault
Tags: hashicorp, vault, security, programming
Published: 2023-12-13 21:10:31

First, a confession. I am not a _software developer_, or a _programmer_. I enjoy writing code, and I am confident in using Python, Go, and TypeScript. I am uncomfortable using Haskell, but I am eager to learn more.

What I am is a _cloud architect_. I work more in the interface between code and cloud services.

With that confession out of the way, let me move on. A few months ago I spent some intense time learning **HashiCorp Vault**, enough to feel confident in using Vault day-to-day and to later pass the [associate certification](https://www.credly.com/badges/515fbaa5-c38d-4f06-807b-8a63737e7d3c/public_url).

Summarizing the Vault certification in four large parts we have:

- Vault architecture
- Vault CLI
- Vault UI
- Vault API

This is very simplified! It would be intimidating if this was the official list of things to learn to pass the exam.

The UI and API part of the exam is small, but the CLI part is huge. You need to know the commands. Luckily they are easy to learn.

During my learning journey I stumbled upon a missing feature in the CLI. In the [Vault documentation for token accessors](https://developer.hashicorp.com/vault/docs/concepts/tokens#token-accessors) I read that you can do the following things with the accessor:

> When tokens are created, a token accessor is also created and returned. This accessor is a value that acts as a reference to a token and can only be used to perform limited actions:
>
> 1. Look up a token's properties (not including the actual token ID)
> 1. Look up a token's capabilities on a path
> 1. Renew the token
> 1. Revoke the token

I tried to perform these actions using the CLI and discovered that it was not possible to list a token's capabilities on a path using its accessor (point 2 in the list above).

If it would have worked the command should have looked like this:

```console
$ vault token capabilities -accessor <accessor value> /the/path
```

But it did not work! I wrote this discrepancy down in my list of notes for later.

## Now it is "later"

I have not made any contributions to large collaborative projects like HashiCorp Vault before, so I was a bit hesitant to even get started.

As with any open-source[^opensource] or source-available projects, it is a good idea to start reading the `CONTRIBUTING.md` file in the repository to see if there are any special guidelines to follow. For Vault (and I assume it is similar for other HashiCorp projects) there were a few things to keep in mind:

### Connect your pull-request to an open issue

The first guideline to follow was to make sure you work on something that has an associated issue:

> When submitting a PR you should reference an existing issue. If no issue already exists, please create one. This can be skipped for trivial PRs like fixing typos.

I searched the list of open issues for anything related to token capabilities. A few issues came up but nothing to do with what I wanted to implement concerning accessors. So I created my own issue (or _feature request_ in the form of an issue).

To be honest, I waited with this part until I was done implementing the feature. I wanted to make sure I would be able to deliver a solution!

My issue: [#24478](https://github.com/hashicorp/vault/issues/24478).

### Describe your work

I confess I cheated a bit with this guideline. This guideline asked you to describe your work in the pull-request:

> Your pull request should have a description of what it accomplishes, how it does so, and why you chose the approach you did.

I took inspiration from other PRs where the description just included a link to the issue it solves. The issue itself contains enough details to describe what is going on. I guess DRY (don't repeat yourself) also applies to issues and pull-requests.

### Include tests

As any good developer know you should test your code:

> PRs should include unit tests that validate correctness and the existing tests must pass.

Of course you should add tests for the functionality you are adding. I took inspiration from the tests that were already written for the `token capabilities` command.

I added tests to verify that the correct number of arguments were provided to the command if the `-accessor` flag was added. I also added a test to verify that the correct capabilities were returned if a valid accessor was used.

### Add a changelog entry

The last guideline to follow was to add a specific file describing the change:

> Please include a file within your PR named `changelog/#.txt`, where `#` is your pull request ID. There are many examples under changelog, but the general format is
>```
>release-note:CATEGORY
>COMPONENT: summary of change
>```

After submitting the pull-request I copied the number it was given and created a new file named `changelog/24479.txt` with a summary of my change.

## The change

So what did I change? Let me tell you!

I did not have to spend hours and hours understanding the structure of the code, it only took me a few minutes to find where I should implement my change. I wanted to update the `token capabilities` **command**, so I opened the **command** directory and found the `token_capabilities.go` file. It's almost like professional software developers have created this codebase.

First of all I had to add the `accessor` flag to the command. I did this in the `TokenCapabilitiesCommand` type:

```go
type TokenCapabilitiesCommand struct {
	*BaseCommand

    // add this
	flagAccessor bool
}
```

I had to update the help text for the command to reflect the added flag. I got some feedback from a Vault developer here, read more about that in the next section.

Next up was to update the `Flags()` method on the `TokenCapabilitiesCommand` type. Before my change it looked like this:

```go
func (c *TokenCapabilitiesCommand) Flags() *FlagSets {
	return c.flagSet(FlagSetHTTP | FlagSetOutputFormat)
}
```

I updated it to the following:

```go
func (c *TokenCapabilitiesCommand) Flags() *FlagSets {
	set := c.flagSet(FlagSetHTTP | FlagSetOutputFormat)

	f := set.NewFlagSet("Command Options")

	f.BoolVar(&BoolVar{
		Name:       "accessor",
		Target:     &c.flagAccessor,
		Default:    false,
		EnvVar:     "",
		Completion: complete.PredictNothing,
		Usage:      "Treat the argument as an accessor instead of a token.",
	})

	return set
}
```

Here I specified details about the accessor flag and text that will appear when you use the help command.

Next up I needed to update the `Run` method on the `TokenCapabilitiesCommand` type. It was using a simple `switch` statement to check for the number of arguments provided, and taking different actions depending on how many. I needed to instead use a more complex switch statement:

```go
switch {
case c.flagAccessor && len(args) < 2:
    c.UI.Error(fmt.Sprintf("Not enough arguments with -accessor (expected 2, got %d)", len(args)))
    return 1
case c.flagAccessor && len(args) > 2:
    c.UI.Error(fmt.Sprintf("Too many arguments with -accessor (expected 2, got %d)", len(args)))
    return 1
case len(args) == 0:
    c.UI.Error("Not enough arguments (expected 1-2, got 0)")
    return 1
case len(args) == 1:
    path = args[0]
case len(args) == 2:
    token, path = args[0], args[1]
default:
    c.UI.Error(fmt.Sprintf("Too many arguments (expected 1-2, got %d)", len(args)))
    return 1
}
```

Some of this code was present in the previous implementation, but to summarize my change I had to check if the `-accessor` flag was provided to the command, and if so I needed to check that the correct number of arguments was provided.

Further down in the `Run` method there was the following check:

```go
if token == "" {
    capabilities, err = client.Sys().CapabilitiesSelf(path)
} else {
    capabilities, err = client.Sys().Capabilities(token, path)
}
```

This just checked if a token was provided or not. If it was not provided, it used the `CapabilitiesSelf` method to see the capabilities of the current token. If a token was provided it instead used the `Capabilities` method to check the capabilities for the provided token. With the `-accessor` flag in the mix I had to add a third case. I needed to update this from an `if`-`else` check to another `switch` statement:

```go
switch {
case token == "":
    capabilities, err = client.Sys().CapabilitiesSelf(path)
case c.flagAccessor:
    capabilities, err = client.Sys().CapabilitiesAccessor(token, path)
default:
    capabilities, err = client.Sys().Capabilities(token, path)
}
```

This change included the case where a token was not provided but the `-accessor` flag was provided. Note that if the `-accessor` flag was included, there is a call to `client.Sys().CapabilitiesAccessor(...)`. I had to implement this `CapabilitiesAccessor` method!

The home for that code was in `api/sys_capabilities.go`.

First of all I added the `CapabilitiesAccessor` method:

```go
func (c *Sys) CapabilitiesAccessor(accessor, path string) ([]string, error) {
	return c.CapabilitiesAccessorWithContext(context.Background(), accessor, path)
}
```

This is just a wrapper for the `CapabilitiesAccessorWithContext` method, with the context argument added.

I also implement the `CapabilitiesAccessorWithContext` method:

```go
func (c *Sys) CapabilitiesAccessorWithContext(ctx context.Context, accessor, path string) ([]string, error) {
	ctx, cancelFunc := c.c.withConfiguredTimeout(ctx)
	defer cancelFunc()

	body := map[string]string{
		"accessor": accessor,
		"path":     path,
	}

	reqPath := "/v1/sys/capabilities-accessor"

	r := c.c.NewRequest(http.MethodPost, reqPath)
	if err := r.SetJSONBody(body); err != nil {
		return nil, err
	}

	resp, err := c.c.rawRequestWithContext(ctx, r)
	if err != nil {
		return nil, err
	}
	defer resp.Body.Close()

	secret, err := ParseSecret(resp.Body)
	if err != nil {
		return nil, err
	}
	if secret == nil || secret.Data == nil {
		return nil, errors.New("data from server response is empty")
	}

	var res []string
	err = mapstructure.Decode(secret.Data[path], &res)
	if err != nil {
		return nil, err
	}

	if len(res) == 0 {
		_, ok := secret.Data["capabilities"]
		if ok {
			err = mapstructure.Decode(secret.Data["capabilities"], &res)
			if err != nil {
				return nil, err
			}
		}
	}

	return res, nil
}
```

I won't go into details of the code, it is mostly a wrapper for calling the API which was already implemented (lucky me!) If it had not been implemented there would have been a lot more work, and probably not something I could have completed on my own.

Now I was done. I did add tests for this change as well, since it was one of the guidelines to follow. I won't cover the tests here.

## Feedback from Vault developers

I got a wonderful comment from one Vault developer (you can read the conversation in the [PR](https://github.com/hashicorp/vault/pull/24479)):

> Looks good.

Then I got some feedback on what the help text for the command should be. I think the suggested changes was good, and accepted them without hesitation.

I am glad the code I wrote was acceptable!

## The result

I want to summarize my work with a famous quote:

> That's one small step for man, one giant leap for mankind.

Replace _mankind_ with _Mattias_ in this quote from Niel Armstrong and you have the feeling I am experiencing right now.

It was a small change. But a huge boost in confidence for me.

I have no plans to be a recurring contributor to Vault, or any other HashiCorp codebase. But this was a great experience and if I stumble upon something else I think is missing I will give it a go!

But, am I a software developer now?

[^opensource]: Mentioning _open source_ and _HashiCorp_ in the same blog post will probably trigger some of you, but I know HashiCorp products are no longer open-source. It does not matter to me, but please let me know why it matters to you!