Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InfluxDB - Add username customization #11796

Merged
merged 3 commits into from
Jun 9, 2021
Merged

Conversation

MilenaHC
Copy link
Contributor

@MilenaHC MilenaHC commented Jun 8, 2021

Adds the ability to customize username generation for dynamic users in InfluxDB.

Uses the new field username_template with the go template language.

@MilenaHC MilenaHC requested a review from a team June 8, 2021 20:28
@hashicorp-cla
Copy link

hashicorp-cla commented Jun 8, 2021

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@tomhjp tomhjp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@kalafut
Copy link
Contributor

kalafut commented Jun 9, 2021

Nice! Will this be for 1.8? If so, please add the 1.8 Milestone to the PR.

@pcman312 pcman312 added this to the 1.8 milestone Jun 9, 2021
Copy link
Member

@austingebauer austingebauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Great job 👍

@@ -3,11 +3,11 @@ package influxdb
import (
"context"
"fmt"
"github.com/hashicorp/vault/sdk/helper/template"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may not have set this up already, but the goimports tool is useful to organize imports in a consistent way. I suggest to try running that tool on this file! I would expect to see this import grouped with the other group ("github.com/*") below.

This is just a stylistic suggestion. We usually try to organize imports consistently in Vault.

Copy link
Member

@calvn calvn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thoughts on imports as Austin mentioned, but otherwise looks great!

@vercel vercel bot temporarily deployed to Preview – vault-storybook June 9, 2021 20:50 Inactive
@vercel vercel bot temporarily deployed to Preview – vault June 9, 2021 20:50 Inactive
@MilenaHC MilenaHC merged commit 1b2f572 into master Jun 9, 2021
@calvn calvn deleted the influxdb-username-customization branch June 9, 2021 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants