Extend enterprise install to also create the license secret #118

tnthornton · 2021-09-15T23:35:55Z

Description of your changes

Add functionality to query Upbound Registry to authenticate the license request, prior to retrieving the License access key and signature from DMV.

Once we have the license access key, a new secret is created in the target namespace to enable license checks for enterprise.

Fixes #115

I have:

Read and followed Upbound's contribution process.
Run make reviewable to ensure this PR is ready for review.
Added backport release-x.y labels to auto-backport this PR, as appropriate.

How has this code been tested

go run ./cmd/... enterprise install v1.0.0
License ID: taylor
Token:

kubectl -n upbound-enterprise get secrets upbound-enterprise-license
NAME                         TYPE     DATA   AGE
upbound-enterprise-license   Opaque   2      133m

Signed-off-by: Taylor Thornton <taylor@upbound.io>

Upbound-CLA · 2021-09-15T23:35:59Z

All committers have signed the CLA.

go.mod

tnthornton · 2021-09-15T23:38:50Z

go.mod

 	github.com/upbound/up-sdk-go v0.1.0
 	golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d
-	helm.sh/helm/v3 v3.6.3
+	helm.sh/helm/v3 v3.6.1-0.20210719095234-663c5698878c


This is pinned to workaround: helm/helm@663c569 not being in a non-rc version that go.mod will pull.

Actually it looks like 3.7.0 was released today. I'll try using that version.

@hasheddan this should answer #118 (comment).

@tnthornton sorry I am not sure I follow why the commit you referenced is needed for the changes in this PR?

cmd/up/enterprise/install.go

Signed-off-by: Taylor Thornton <taylor@upbound.io>

hasheddan

Thanks for this great work @tnthornton! A few questions / comments below :)

cmd/up/enterprise/install.go

hasheddan · 2021-09-20T09:57:11Z

go.mod

-	k8s.io/apimachinery v0.21.0
-	k8s.io/cli-runtime v0.21.0
-	k8s.io/client-go v0.21.0
+	helm.sh/helm/v3 v3.7.0


I don’t think this will cause any issues, and I like staying up to date with latest release, but was there something in this PR that required a bump?

internal/license/dmv.go

hasheddan

@tnthornton I removed the backport-0.4 label as this one should just go into main.

Signed-off-by: Taylor Thornton <taylor@upbound.io>

tnthornton · 2021-09-20T21:39:54Z

With the latest changes:

*[enterprise-install-auth][~/codez/up]$ go run ./cmd/... enterprise install --repo registry.upbound.io/enterprise-dev v0.0.0-119.g7bd3967 --key-version-override v0.0.0 --org-id enterprise-dev --dmv http://localhost:8080
License ID: <OMITTED>
Token:
up: error: unable to build kubernetes objects from release manifest: [unable to recognize "": no matches for kind "Certificate" in version "cert-manager.io/v1alpha2", unable to recognize "": no matches for kind "Issuer" in version "cert-manager.io/v1alpha2"]
exit status 1

*[enterprise-install-auth][~/codez/up]$ go run ./cmd/... enterprise install --repo registry.upbound.io/enterprise-dev v0.0.0-119.g7bd3967 --skip-license
License ID: <OMITTED>
Token:
up: error: unable to build kubernetes objects from release manifest: [unable to recognize "": no matches for kind "Certificate" in version "cert-manager.io/v1alpha2", unable to recognize "": no matches for kind "Issuer" in version "cert-manager.io/v1alpha2"]
exit status 1

hasheddan · 2021-09-20T21:47:36Z

cmd/up/enterprise/install.go


 	Version string `arg:"" help:"Enterprise version to install."`

-	Repo *url.URL `hidden:"" env:"ENTERPRISE_REPO" default:"registry.upbound.io/enterprise" help:"Set repo for enterprise."`
+	LicenseSecretName string `default:"upbound-enterprise-license" help:"Name of secret that will be populated with license data."`
+	SkipLicense       bool   `hidden:"" help:"Skip providing a license for enteprise install"`


nitpick for consistency:

Suggested change

SkipLicense bool `hidden:"" help:"Skip providing a license for enteprise install"`

SkipLicense bool `hidden:"" help:"Skip providing a license for enteprise install."`

hasheddan · 2021-09-20T21:53:50Z

internal/license/dmv.go

+}
+
+// NewProvider constructs a new dmv provider
+func NewProvider(modifiers ...ProviderModifierFn) Provider {


Prefer returning *dmv

hasheddan · 2021-09-20T21:54:47Z

internal/auth/registry.go

+	}
+}
+
+func (u *upboundRegistry) GetToken(ctx context.Context) (Response, error) {


I would typically return a pointer here (nil in error case)

Suggested change

func (u *upboundRegistry) GetToken(ctx context.Context) (Response, error) {

func (u *upboundRegistry) GetToken(ctx context.Context) (*Response, error) {

hasheddan · 2021-09-20T21:55:39Z

internal/license/dmv.go

+	}
+}
+
+func (d *dmv) GetAccessKey(ctx context.Context, token, version string) (Response, error) {


Same comment about returning *Response here

hasheddan · 2021-09-20T22:00:22Z

cmd/up/enterprise/install.go

+			secret,
+			metav1.UpdateOptions{},
+		); err != nil {
+			return errors.Wrap(err, errCreateLicenseSecret)


I think this will end up getting double wrapped by caller -- You also could likely skip this check and return errors.Wrap of err below as I believe it returns nil if error is nil.

hasheddan

Nice work @tnthornton! 🚀

add version overrides for dev Signed-off-by: Taylor Thornton <taylor@upbound.io>

Extend enterprise install to also create the license secret

6245287

Signed-off-by: Taylor Thornton <taylor@upbound.io>

tnthornton added the backport release-0.4 label Sep 15, 2021

pull-request-size bot added the size/XL label Sep 15, 2021

tnthornton commented Sep 15, 2021

View reviewed changes

go.mod Outdated Show resolved Hide resolved

tnthornton commented Sep 15, 2021

View reviewed changes

cmd/up/enterprise/install.go Outdated Show resolved Hide resolved

tnthornton requested a review from hasheddan September 15, 2021 23:41

tnthornton added 2 commits September 15, 2021 16:49

use latest helm.sh/helm/v3

388b6b6

Signed-off-by: Taylor Thornton <taylor@upbound.io>

converge dev versions to major.minor.patch

c65072a

Signed-off-by: Taylor Thornton <taylor@upbound.io>

tnthornton force-pushed the enterprise-install-auth branch from d2a3754 to c65072a Compare September 16, 2021 00:43

hasheddan reviewed Sep 20, 2021

View reviewed changes

tnthornton requested a review from hasheddan September 20, 2021 17:48

hasheddan removed the backport release-0.4 label Sep 20, 2021

hasheddan reviewed Sep 20, 2021

View reviewed changes

use hand rolled client for dmv rather than generated client

64118f4

Signed-off-by: Taylor Thornton <taylor@upbound.io>

tnthornton force-pushed the enterprise-install-auth branch from 6c33a80 to 64118f4 Compare September 20, 2021 20:51

tnthornton requested a review from hasheddan September 20, 2021 21:40

hasheddan reviewed Sep 20, 2021

View reviewed changes

tnthornton force-pushed the enterprise-install-auth branch 2 times, most recently from e9b5f12 to e1aa08a Compare September 20, 2021 23:09

hasheddan approved these changes Sep 20, 2021

View reviewed changes

tnthornton force-pushed the enterprise-install-auth branch from e1aa08a to 8336bfd Compare September 20, 2021 23:15

make license install conditional

4d4e081

add version overrides for dev Signed-off-by: Taylor Thornton <taylor@upbound.io>

tnthornton force-pushed the enterprise-install-auth branch from 8336bfd to 4d4e081 Compare September 20, 2021 23:23

tnthornton merged commit 53a30b1 into upbound:main Sep 20, 2021

tnthornton deleted the enterprise-install-auth branch September 20, 2021 23:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend enterprise install to also create the license secret #118

Extend enterprise install to also create the license secret #118

tnthornton commented Sep 15, 2021

Upbound-CLA commented Sep 15, 2021 •

edited

Loading

tnthornton Sep 15, 2021 •

edited

Loading

tnthornton Sep 15, 2021

tnthornton Sep 20, 2021

hasheddan Sep 20, 2021

hasheddan left a comment

hasheddan Sep 20, 2021

hasheddan left a comment

tnthornton commented Sep 20, 2021

hasheddan Sep 20, 2021

hasheddan Sep 20, 2021

hasheddan Sep 20, 2021

hasheddan Sep 20, 2021

hasheddan Sep 20, 2021

hasheddan left a comment

	SkipLicense bool `hidden:"" help:"Skip providing a license for enteprise install"`
	SkipLicense bool `hidden:"" help:"Skip providing a license for enteprise install."`

	func (u *upboundRegistry) GetToken(ctx context.Context) (Response, error) {
	func (u upboundRegistry) GetToken(ctx context.Context) (Response, error) {

Extend enterprise install to also create the license secret #118

Extend enterprise install to also create the license secret #118

Conversation

tnthornton commented Sep 15, 2021

Description of your changes

How has this code been tested

Upbound-CLA commented Sep 15, 2021 • edited Loading

tnthornton Sep 15, 2021 • edited Loading

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

hasheddan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hasheddan left a comment

Choose a reason for hiding this comment

tnthornton commented Sep 20, 2021

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

hasheddan left a comment

Choose a reason for hiding this comment

Upbound-CLA commented Sep 15, 2021 •

edited

Loading

tnthornton Sep 15, 2021 •

edited

Loading