Add CLI instructions as required by the Marketplace #22

maelvls · 2021-02-16T18:22:46Z

Closes #21

The Marketplace requires us to give a "Deploy documentation URL" that details the steps for installing the solution from the command line.

I took a lot of inspiration from the Unleash Enterprise solution (which is a paid offering). Our own CLI deploy documentation will probably look like something similar to their README.md.

⚠️ If we want to be able to share the CLI doc URL, we must set this repo as public

Signed-off-by: Maël Valais [email protected]

The Marketplace requires us to give a "Deploy documentation URL" that details the steps for installing the solution from the command line. Signed-off-by: Maël Valais <[email protected]>

maelvls · 2021-02-17T08:15:34Z

Update:

As detailed in the Marketplace doc, the CLI instructions I wrote will not create RBAC or service accounts except for the google-cas-issuer service account since it is required for a subsequent step in the README
I took all my inspiration from the Unleash Enterprise readme

Signed-off-by: Maël Valais <[email protected]>

wallrj

Review Part 1

wallrj · 2021-02-17T10:28:03Z

README.md

+The [workload
+identity](https://cloud.google.com/kubernetes-engine/docs/how-to/workload-identity)
+must be enabled on your cluster. To create a cluster that has _workload
+identity_ feature enabled, run the following command:


Suggested change

The [workload

identity](https://cloud.google.com/kubernetes-engine/docs/how-to/workload-identity)

must be enabled on your cluster. To create a cluster that has _workload

identity_ feature enabled, run the following command:

The [workload

identity](https://cloud.google.com/kubernetes-engine/docs/how-to/workload-identity) feature

must be enabled on your cluster. To create a cluster that has _workload

identity_ feature enabled, run the following command:

wallrj · 2021-02-17T11:34:53Z

README.md

+Choose an instance name and
+[namespace](https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/)
+for the application. In most cases, you can use the `default` namespace.


Suggested change

Choose an instance name and

[namespace](https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/)

for the application. In most cases, you can use the `default` namespace.

Choose an instance name and

[namespace](https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/)

for the application.

I don't think we want users to install cert-manager in the default namespace.
cert-manager will use it's namespace to determine where to look for secrets for cluster scoped ClusterIssuers.
cert-manager usually lives in cert-manager.

But at the same time I don't know whether it's appropriate to also install cas-issuer and preflight agent in that namespace.

Good point, this wording was for Unleash Enterprise and in their case they did recommend the app to be running on a standalone cluster.

In the case of Jetstack Secure for cert-manager, running it in default is a bad idea 😅

wallrj · 2021-02-17T11:39:04Z

README.md

+TAG="1.1.0-gcm.1"        # stable tag (recommended)
+TAG="1.1.0"              # moving tag, targets cert-manager 1.1.0
+TAG="1.1"                # moving tag, targets cert-manager 1.1
+```


This would confuse me. How should I decide which tag to use? I would need to set the image pull policy to always and to do kubectl rollout restart on cert-manager, preflight, cas-issuer deployments in order to get the latest versions from the moving-target tags.

Perhaps we should remove any reference to those moving target tags until we figure out the correct upgrade process.

wallrj · 2021-02-17T11:45:04Z

README.md

+```
+
+The available tags are listed on the [Marketplace Container
+Registry](marketplace.gcr.io/jetstack-public/jetstack-secure-for-cert-manager).


Again, what should the user do with this info?
Instead, shall we hard code a version tag for now into this README and create a git tag, and link to that tag from the GCM page?
When we release a new version of jsp-gcm, we update the README with the latest tag, and update the GCM page with the link to that tag.

Ignore these comments if they don't make sense.

yeah, I thinnk i didnt't think twice while writing this; I am not helping the user pick the right version here

I'll remove this link for now and let's figure out a way to let users know which version they should pick later.

wallrj · 2021-02-17T11:49:19Z

README.md

+```shell
+APP_INSTANCE_NAME=jetstack-secure-1
+NAMESPACE=default
+```


Maybe add a command / instruction, to create the namespace

wallrj · 2021-02-17T11:51:41Z

README.md

+```shell
+APP_INSTANCE_NAME=jetstack-secure-1
+NAMESPACE=default
+```


Suggested change

```shell

APP_INSTANCE_NAME=jetstack-secure-1

NAMESPACE=default

```

```shell

APP_INSTANCE_NAME=jetstack-secure-1

NAMESPACE=default

kubectl create namespace "${NAMESPACE}" || true

wallrj · 2021-02-17T11:55:08Z

README.md

+```shell
+kubectl apply -f "${APP_INSTANCE_NAME}_manifest.yaml" --namespace "${NAMESPACE}"
+```


The manifest contains various references to kube-system which clashes with the namespace given to kubectl

$ kubectl apply -f "${APP_INSTANCE_NAME}_manifest.yaml" --namespace "${NAMESPACE}" ... the namespace from the provided object "kube-system" does not match the namespace "jetstack-secure". You must pass '--namespace=kube-system' to perform this operation. the namespace from the provided object "kube-system" does not match the namespace "jetstack-secure". You must pass '--namespace=kube-system' to perform this operation. the namespace from the provided object "kube-system" does not match the namespace "jetstack-secure". You must pass '--namespace=kube-system' to perform this operation. the namespace from the provided object "kube-system" does not match the namespace "jetstack-secure". You must pass '--namespace=kube-system' to perform this operation.

Signed-off-by: Maël Valais <[email protected]>

maelvls · 2021-02-17T12:17:06Z

Done! I moved our own doc to docs/TESTING-DEPLOYER.md so there is no confusion between the CLI instructions and the UI-specific deployer stuff

Signed-off-by: Maël Valais <[email protected]>

wallrj

Looking good to me @maelvls

You've put a lot of work in to this!

Some of the instructions failed for me, but perhaps that's down to my version of gcloud or my Linux environment.

Please answer or address the comments below, and then I'll add a lgtm

wallrj · 2021-02-17T12:04:20Z

README.md

+The Certificate Authority Service is a highly available, scalable Google Cloud
+service that enables you to simplify, automate, and customize the
+deployment, management, and security of private certificate authorities
+(CA).


Suggested change

The Certificate Authority Service is a highly available, scalable Google Cloud

service that enables you to simplify, automate, and customize the

deployment, management, and security of private certificate authorities

(CA).

[Google Certificate Authority Service][] is a highly available, scalable Google Cloud

service that enables you to simplify, automate, and customize the

deployment, management, and security of private certificate authorities

(CA).

[Google Certificate Authority Service]: https://cloud.google.com/certificate-authority-service/

wallrj · 2021-02-17T12:07:05Z

README.md

+```sh
+gcloud beta privateca roots create my-ca --location $LOCATION --subject="CN=root,O=my-ca"
+gcloud beta privateca subordinates create my-sub-ca  --issuer=my-ca --location $LOCATION --subject="CN=intermediate,O=my-ca,OU=my-sub-ca"
+```


Suggested change

```sh

gcloud beta privateca roots create my-ca --location $LOCATION --subject="CN=root,O=my-ca"

gcloud beta privateca subordinates create my-sub-ca --issuer=my-ca --location $LOCATION --subject="CN=intermediate,O=my-ca,OU=my-sub-ca"

```

```sh

LOCATION=europe-west1

gcloud beta privateca roots create my-ca --location ${LOCATION} --subject="CN=root,O=my-ca"

gcloud beta privateca subordinates create my-sub-ca --issuer=my-ca --location ${LOCATION} --issuer-location ${LOCATION} --subject="CN=intermediate,O=my-ca,OU=my-sub-ca"

Because I got this error message:

[richard@drax jsp-gcm]$ gcloud beta privateca subordinates create my-sub-ca --issuer=my-ca --location ${LOCATION} --subject="CN=intermediate,O=my-ca,OU=my-sub-ca" ERROR: (gcloud.beta.privateca.subordinates.create) Error parsing [issuer]. The [Issuer] resource is not properly specified. Failed to find attribute [location]. The attribute can be set in the following ways: - provide the argument `--issuer-location` on the command line - set the property `privateca/location`

And even with that extra --issuer-location ... argument, I now get this error:

[richard@drax jsp-gcm]$ gcloud beta privateca subordinates create my-sub-ca --issuer=my-ca --location ${LOCATION} --issuer-location ${LOCATION} --subject="CN=intermediate,O=my-ca,OU=my-sub-ca" WARNING: CA Service is currently in preview. Please remember that all resources created during preview will be deleted when CA service transitions to General Availability (GA). Relying on these certificate authorities for production traffic is discouraged. Creating Certificate Authority....done. Activating CA....failed. ERROR: (gcloud.beta.privateca.subordinates.create) Could not verify certificate with chain: Chain violates pathlength restriction. Pathlength 0 on cert CN=root,O=my-ca with CA length 1

Perhaps we should just link to Google CAS Issuer installation instructions instead of recreating them here.

Yeah... I agree, I'll just give a link to the official documentation.

The new command would be:

LOCATION=europe-west1 gcloud beta privateca roots create example-root-ca \ --location ${LOCATION} \ --reusable-config "example-intermediate-ca-pathlen-0" \ --subject="CN=Example Root CA 1, O=Example LLC" gcloud beta privateca subordinates create example-ca-1 \ --issuer=example-root-ca \ --issuer-location ${LOCATION} \ --reusable-config "example-intermediate-ca-pathlen-0" \ --subject="CN=Example Intermediate CA, O=Example LLC"

I think the subject validation appeared recently maybe? Do they have some form of CHANGELOG for the Google CAS service btw? @jakexks

wallrj · 2021-02-17T12:16:00Z

README.md

+This is the repository that holds the configuration for our Google
+Marketplace solution, [jetstack-secure-for-cert-manager][].


Suggested change

This is the repository that holds the configuration for our Google

Marketplace solution, [jetstack-secure-for-cert-manager][].

This is in the wrong place.

wallrj · 2021-02-17T12:16:47Z

README.md

+
 ## Technical considerations

 **Retagging cert-manager images:**


Shall we remove all this stuff now? It may confuse the user who visits this repo looking for GCM CLI instructions.

Yes, good idea; I moved it to docs/TESTING-DEPLOYER.md, does it make sense? I also added a note about the "deployer" being only for the UI stuff

wallrj · 2021-02-17T12:18:49Z

chart/jetstacksecure-mp/values.yaml

+  acmesolver:
+    image:
+      repository: marketplace.gcr.io/jetstack-public/jetstack-secure-for-cert-manager/cert-manager-acmesolver
+      tag:


Suggested change

tag:

tag: 1.1.0-gcm.1

Is this supposed to have the same tag as other images?

Co-authored-by: Richard Wall <[email protected]>

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Richard Wall <[email protected]>

Signed-off-by: Maël Valais <[email protected]>

maelvls · 2021-02-17T14:13:18Z

Looks liike some objects have no namespace field which means they get created in the default namespace:

Signed-off-by: Richard Wall <[email protected]>

The "cert-manager" namespace made sense for cert-manager itself but we are now talking about "jetstack-secure" Signed-off-by: Maël Valais <[email protected]>

Signed-off-by: Maël Valais <[email protected]>

Signed-off-by: Richard Wall <[email protected]>

And add an icon! Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Richard Wall <[email protected]>

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Richard Wall <[email protected]>

wallrj

/lgtm

jetstack-bot added the size/L label Feb 16, 2021

maelvls marked this pull request as draft February 16, 2021 18:23

jetstack-bot added the do-not-merge/work-in-progress label Feb 16, 2021

maelvls added this to the initial-release milestone Feb 16, 2021

Add CLI instructions as required by the Marketplace

441c97d

The Marketplace requires us to give a "Deploy documentation URL" that details the steps for installing the solution from the command line. Signed-off-by: Maël Valais <[email protected]>

maelvls force-pushed the add-cli-instructions branch from eceb144 to 441c97d Compare February 17, 2021 08:06

Helm: default to 1.1.0-gcm.1 and the marketplace repo

b591046

Signed-off-by: Maël Valais <[email protected]>

maelvls mentioned this pull request Feb 17, 2021

Fill the "Solution description" section on the solution page #17

Closed

8 tasks

Readme: fill in the description

8f89050

Signed-off-by: Maël Valais <[email protected]>

maelvls mentioned this pull request Feb 17, 2021

Click on "Create version" in the Marketplace UI #16

Closed

Readme: add a "download license" step

412f629

Signed-off-by: Maël Valais <[email protected]>

wallrj requested changes Feb 17, 2021

View reviewed changes

Readme: moved the "testing deployer" doc to docs/TESTING-DEPLOYER.md

f643fa9

Signed-off-by: Maël Valais <[email protected]>

jetstack-bot added size/XL and removed size/L labels Feb 17, 2021

Docs: mention that the deployer image is ONLY used by the UI

bf5eff5

Signed-off-by: Maël Valais <[email protected]>

maelvls marked this pull request as ready for review February 17, 2021 12:19

jetstack-bot removed the do-not-merge/work-in-progress label Feb 17, 2021

wallrj requested changes Feb 17, 2021

View reviewed changes

maelvls and others added 7 commits February 17, 2021 13:29

Signed-off-by: Maël Valais <[email protected]>

752daef

Co-authored-by: Richard Wall <[email protected]>

PR comments: remove confusing 1.1, 1.1.0, 1.1.0-gcm.1

2f87395

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Richard Wall <[email protected]>

PR comment: kubectl apply should not have --namespace

9380b22

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Richard Wall <[email protected]>

PR comment: remove the often-changing subordinate creation

fd89c99

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Richard Wall <[email protected]>

Re-enable the preflight deployment

4a95d03

Signed-off-by: Maël Valais <[email protected]>

Add a note about why we set preflight.replicaCount to 0

5f35f4e

Signed-off-by: Maël Valais <[email protected]>

Rename repo from jsp-gcm to jetstack-secure-gcm

b83bdb7

Signed-off-by: Maël Valais <[email protected]>

Increase the deployer wait timeout in logs-deployer and logs-smoketest

c5a4c9f

Signed-off-by: Richard Wall <[email protected]>

Use default namespace "jetstack-secure"

333ba6d

The "cert-manager" namespace made sense for cert-manager itself but we are now talking about "jetstack-secure" Signed-off-by: Maël Valais <[email protected]>

maelvls force-pushed the add-cli-instructions branch from d760637 to 331daad Compare February 17, 2021 14:35

Helm: resources were wrongly created in "default"

65b645b

Signed-off-by: Maël Valais <[email protected]>

maelvls force-pushed the add-cli-instructions branch from 331daad to 65b645b Compare February 17, 2021 14:38

wallrj and others added 4 commits February 17, 2021 14:43

Fix the smoke-test-pod name in log-smoke-test job

2f2bb4d

Signed-off-by: Richard Wall <[email protected]>

Wait for 2m for the test Certificate to be ready

cb46696

Signed-off-by: Richard Wall <[email protected]>

Use app.kubernetes.io/instance for the application

3529f27

And add an icon! Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Richard Wall <[email protected]>

Add the optional step "Set up Jetstack Secure agent"

212bbbb

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Richard Wall <[email protected]>

maelvls force-pushed the add-cli-instructions branch from 1a31d10 to 212bbbb Compare February 17, 2021 16:58

wallrj approved these changes Feb 17, 2021

View reviewed changes

jetstack-bot assigned wallrj Feb 17, 2021

jetstack-bot added the lgtm label Feb 17, 2021

jetstack-bot merged commit 7e19d08 into main Feb 17, 2021

maelvls deleted the add-cli-instructions branch November 3, 2021 09:40

-```sh
-gcloud beta privateca roots create my-ca --location $LOCATION --subject="CN=root,O=my-ca"
-gcloud beta privateca subordinates create my-sub-ca  --issuer=my-ca --location $LOCATION --subject="CN=intermediate,O=my-ca,OU=my-sub-ca"
-```
+```sh
+LOCATION=europe-west1
+gcloud beta privateca roots create my-ca --location ${LOCATION} --subject="CN=root,O=my-ca"
+gcloud beta privateca subordinates create my-sub-ca  --issuer=my-ca --location ${LOCATION}  --issuer-location ${LOCATION} --subject="CN=intermediate,O=my-ca,OU=my-sub-ca"

		This is the repository that holds the configuration for our Google
		Marketplace solution, [jetstack-secure-for-cert-manager][].


		## Technical considerations

		Retagging cert-manager images:

Add CLI instructions as required by the Marketplace #22

Add CLI instructions as required by the Marketplace #22

Uh oh!

Conversation

maelvls commented Feb 16, 2021

Uh oh!

maelvls commented Feb 17, 2021

Uh oh!

wallrj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maelvls commented Feb 17, 2021

Uh oh!

wallrj left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maelvls commented Feb 17, 2021

Uh oh!

wallrj left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants