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.

Sign up for GitHub

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

INTMDB-232: Fix user agent version #505

Merged
merged 2 commits into from
Aug 5, 2021
Merged

INTMDB-232: Fix user agent version #505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f5defb0
INTMDB-232: Fix user agent version
MihaiBojin Aug 4, 2021
13eb163
fix lint
MihaiBojin Aug 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .goreleaser.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ builds:
flags:
- -trimpath
ldflags:
- -s -w -X mongodbatlas/version.ProviderVersion={{.Version}}
- -s -w -X 'github.com/mongodb/terraform-provider-mongodbatlas/version.ProviderVersion={{.Version}}'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ProviderVersion was not being replaced, without the full repo path being specified

goos:
- freebsd
- windows
Expand Down
4 changes: 2 additions & 2 deletions GNUmakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ GOOPTS="-p 2"

GITTAG=$(shell git describe --always --tags)
VERSION=$(GITTAG:v%=%)
LINKER_FLAGS=-X mongodbatlas/version.ProviderVersion=${VERSION}
LINKER_FLAGS=-s -w -X 'github.com/mongodb/terraform-provider-mongodbatlas/version.ProviderVersion=${VERSION}'

GOLANGCI_VERSION=v1.29.0

Expand Down Expand Up @@ -42,7 +42,7 @@ websitefmtcheck:
lint:
@echo "==> Checking source code against linters..."
# https://github.com/golangci/golangci-lint/issues/337 fixing error
golangci-lint run ./$(PKG_NAME) -v --deadline=30m
bin/golangci-lint run ./$(PKG_NAME) -v --deadline=30m
Copy link
Contributor Author

Choose a reason for hiding this comment

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

drive-by: golangci-lint is installed under bin/ - there's no point relying on the tool being available in the system path. cc @gssbzn in case I'm missing something

Copy link
Collaborator

Choose a reason for hiding this comment

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

line 16 includes the bin folder in the PATH (export PATH := ./bin:$(PATH)) for convenience, no need to revert this just curious if that no longer works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't work for me on Mac, but it worked in GH actions


tools: ## Install dev tools
@echo "==> Installing dependencies..."
Expand Down
7 changes: 4 additions & 3 deletions mongodbatlas/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import (

"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/logging"
"github.com/mongodb/terraform-provider-mongodbatlas/version"

digest "github.com/mongodb-forks/digest"
"github.com/mongodb-forks/digest"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

drive-by fix - unnecessary aliasing

matlasClient "go.mongodb.org/atlas/mongodbatlas"
realmAuth "go.mongodb.org/realm/auth"
"go.mongodb.org/realm/realm"
Expand Down Expand Up @@ -38,7 +39,7 @@ func (c *Config) NewClient(ctx context.Context) (interface{}, diag.Diagnostics)

client.Transport = logging.NewTransport("MongoDB Atlas", transport)

optsAtlas := []matlasClient.ClientOpt{matlasClient.SetUserAgent("terraform-provider-mongodbatlas/" + ProviderVersion)}
optsAtlas := []matlasClient.ClientOpt{matlasClient.SetUserAgent("terraform-provider-mongodbatlas/" + version.ProviderVersion)}
Copy link
Collaborator

@gssbzn gssbzn Aug 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

[q] this is the user agent we do for the cli

var userAgent = fmt.Sprintf("%s/%s (%s;%s)", config.ToolName, version.Version, runtime.GOOS, runtime.GOARCH)

should we make something similar, ie: include os and arch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that makes sense. Will update later, but if you want to make the change directly on the PR it would be even better, since I won't be in front of a computer for the next few hours. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On second thought @gssbzn, if we made this change, the UA would be reported as:

 userAgent: terraform-provider-mongodbatlas/0.9.1-29-g13eb1636 (darwin;amd64) go-mongodbatlas/0.10.1 (darwin;amd64)

(since we add the OS/ARCH via the go client)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to merge the PR as is / I don't think duplicating that info makes sense.

Copy link
Collaborator

Choose a reason for hiding this comment

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

totally forgot we were doing that in the client, merge away

if c.BaseURL != "" {
optsAtlas = append(optsAtlas, matlasClient.SetBaseURL(c.BaseURL))
}
Expand All @@ -50,7 +51,7 @@ func (c *Config) NewClient(ctx context.Context) (interface{}, diag.Diagnostics)
}

// Realm
optsRealm := []realm.ClientOpt{realm.SetUserAgent("terraform-provider-mongodbatlas/" + ProviderVersion)}
optsRealm := []realm.ClientOpt{realm.SetUserAgent("terraform-provider-mongodbatlas/" + version.ProviderVersion)}
if c.BaseURL != "" {
optsRealm = append(optsRealm, realm.SetBaseURL(c.BaseURL))
}
Expand Down
2 changes: 1 addition & 1 deletion mongodbatlas/version.go → version/version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package mongodbatlas
package version
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved under root / following the same directory structure as the AWS provider


var (
// ProviderVersion is set during the release process to the release version of the binary
Expand Down