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

Use jetstack public to build and fix Application wrangling #35

Merged
merged 35 commits into from
Mar 12, 2021

Conversation

maelvls
Copy link
Member

@maelvls maelvls commented Mar 10, 2021

In this PR, I fix a couple of things

  1. Remind ourselves that we should only use jetstack-public in order to run gcloud builds submit, otherwise the verify step will fail (see "hardcoded" in cloudbuild.yml)
  2. Fix the Application component wrangling (cainjector and webhook still missing...). the names are confusion thourh (controller = google-cas-issuer), so we need to work that out
  3. ubbagent is now a side-car to the cert-manager controller deployment
  4. I had to unpack the cert-manager tarball... sorry for the 16 000 new lines!!!

@maelvls
Copy link
Member Author

maelvls commented Mar 10, 2021

I have cut a second release, 1.1.0-gcm.2, that fixes the issues reported in #34> In a few words, the deployments are now properly showing in the application UI. This fix was urgent because the application UI is the first page they will see after having clicked "Deploy"; in 1.1.0-gcm.1, cert-manager would not even show up in the UI. Only the ubbagent was showing, which makes for a very poor experience.

My plan is to click "Submit for review" tonight (probably late), and have Google review 1.1.0-gcm.2 during the night (10-11 March). If Google accepts it during the night, we should be able to make the application public by tomorrow morning.

Last note: I am still very unhappy with 1.1.0-gcm.2 (see below screenshot). Two deployments are missing from the list (even though they are running fine): the cainjector and the webhook. The deployment names are ultra confusing. Also, the icon is broken:

Here is a view of the Application UI after installing the application:

The application page on the Google Kubernetes Engine section of Google Cloud.

step by step process

First I built 1.1.0-gcm.2 deployer:

% gcloud builds submit --project jetstack-public --timeout 1800s --config cloudbuild.yaml \
  --substitutions _CLUSTER_NAME=smoke-test,_CLUSTER_LOCATION=europe-west2-b,_APP_MINOR_VERSION=1.1,_APP_VERSION=1.1.0-gcm.2
  • logs
  • image: gcr.io/jetstack-public/jetstack-secure-for-cert-manager/deployer:1.1.0-gcm.2

Then I retagged the images

retag() {
  docker pull $1 && docker tag $1 $2 && docker push $2
}
retag gcr.io/jetstack-public/jetstack-secure-for-cert-manager:{google-review,1.1.0-gcm.2}
retag gcr.io/jetstack-public/jetstack-secure-for-cert-manager/cert-manager-acmesolver:{google-review,1.1.0-gcm.2}
retag gcr.io/jetstack-public/jetstack-secure-for-cert-manager/cert-manager-cainjector:{google-review,1.1.0-gcm.2}
retag gcr.io/jetstack-public/jetstack-secure-for-cert-manager/cert-manager-webhook:{google-review,1.1.0-gcm.2}
retag gcr.io/jetstack-public/jetstack-secure-for-cert-manager/cert-manager-google-cas-issuer:{google-review,1.1.0-gcm.2}
retag gcr.io/jetstack-public/jetstack-secure-for-cert-manager/preflight:{google-review,1.1.0-gcm.2}
retag gcr.io/cloud-marketplace-tools/metering/ubbagent:latest gcr.io/jetstack-public/jetstack-secure-for-cert-manager/ubbagent:1.1.0-gcm.2

Finally I did (still on the branch... I know, not good)

git tag 1.1.0-gcm.2
git push --tags

Then I updated the version on the UI and clicked "Submit for review".

@maelvls maelvls changed the title Use jetstack public to build Use jetstack public to build and fix Application wrangling Mar 10, 2021
@maelvls maelvls mentioned this pull request Mar 10, 2021
2 tasks
maelvls added 19 commits March 11, 2021 08:17
Both the google cas issuer's deployment, cert-manager and preflight were
using the same deployment name. The issue was created when we chose to
use the same nameOverride for all the subcharts.

In jetstack-secure-gcm/charts/google-cas-issuer/templates/deployment.yaml:

   kind: Deployment
   metadata:
     name: supertestapp-jetstack-secure-gcm-controller
     namespace: "jetstack-secure"
     labels:
       helm.sh/chart: google-cas-issuer-1.0.0
       app.kubernetes.io/name: jetstack-secure-gcm
       app.kubernetes.io/instance: supertestapp
       app.kubernetes.io/component: "controller"
       app.kubernetes.io/version: "0.1.0"
       app.kubernetes.io/managed-by: Helm

In jetstack-secure-gcm/charts/cert-manager/templates/deployment.yaml:

   apiVersion: apps/v1
   kind: Deployment
   metadata:
     name: supertestapp-jetstack-secure-gcm
     namespace: "jetstack-secure"
     labels:
       app: jetstack-secure-gcm
       app.kubernetes.io/name: jetstack-secure-gcm
       app.kubernetes.io/instance: supertestapp
       app.kubernetes.io/managed-by: Helm
       app.kubernetes.io/component: "controller"
       helm.sh/chart: cert-manager-v1.1.0

In jetstack-secure-gcm/charts/preflight/templates/deployment.yaml:

   apiVersion: apps/v1
   kind: Deployment
   metadata:
     name: supertestapp-jetstack-secure-gcm
     namespace: "jetstack-secure"
     labels:
       helm.sh/chart: preflight-0.1.0
       app.kubernetes.io/name: jetstack-secure-gcm
       app.kubernetes.io/instance: supertestapp
       app.kubernetes.io/version: "v0.1.27"
       app.kubernetes.io/managed-by: Helm

Signed-off-by: Maël Valais <[email protected]>
Signed-off-by: Maël Valais <[email protected]>
Purpose: have the correct app.kubernetes.io/name: .Chart.Name across all
objects so that the Application object can "wrangle" all the Kubernetes
components using its label selector.

Signed-off-by: Maël Valais <[email protected]>
@maelvls maelvls force-pushed the use-jetstack-public-to-build branch from 2234ff1 to 8255d97 Compare March 11, 2021 07:21
@maelvls maelvls force-pushed the use-jetstack-public-to-build branch from 8255d97 to 578e7a4 Compare March 11, 2021 08:31
maelvls added 2 commits March 11, 2021 11:51
Signed-off-by: Maël Valais <[email protected]>
@maelvls
Copy link
Member Author

maelvls commented Mar 11, 2021

I have cut another release, 1.1.0-gcm.3. With this last release, we fixed all the issues that we knew of (icon is now showing, deployment names are now way more descriptive, ubbagent is integrated in the main cert-manager pod instead of being a standalone deployment).

My plan is to click "Submit for review" in a few minutes, and have Google review 1.1.0-gcm.3 during the night (11-12 March). If Google accepts it during the night, we should be able to make the application public by tomorrow morning!

step by step process

first I built 1.1.0-gcm.3 deployer

% gcloud builds submit --project jetstack-public --timeout 1800s --config cloudbuild.yaml \
  --substitutions _CLUSTER_NAME=smoke-test,_CLUSTER_LOCATION=europe-west2-b,_APP_MINOR_VERSION=1.1,_APP_VERSION=1.1.0-gcm.3
  • logs
  • image: gcr.io/jetstack-public/jetstack-secure-for-cert-manager/deployer:1.1.0-gcm.3

Finally, I did

git tag 1.1.0-gcm.3

And I went to the UI, updated the image and "Save" and Submit for review.

maelvls added 4 commits March 11, 2021 14:05
I would get the error:

   % kubectl -n $NAMESPACE apply -f agent-config.yaml
   the namespace from the provided object "jetstack-secure" does not match the namespace "test-1". You must pass '--namespace=jetstack-secure' to perform this operation.
   the namespace from the provided object "jetstack-secure" does not match the namespace "test-1". You must pass '--namespace=jetstack-secure' to perform this operation.

I also fixed the rollout command, since we changed the name of the
deployment. I wish the preflight deployment had an label such as:

   app.kubernetes.io/component: preflight

That would be very helpful!

Signed-off-by: Maël Valais <[email protected]>
@maelvls maelvls force-pushed the use-jetstack-public-to-build branch 2 times, most recently from df2d76d to e1300dc Compare March 11, 2021 14:58
The preflight agent was unable to list and get the following objects:

  services
  replicasets
  statefulsets
  jobs
  pods
  daemonsets
  ingresses
  deployments
  cronjobs

Signed-off-by: Maël Valais <[email protected]>
@maelvls maelvls force-pushed the use-jetstack-public-to-build branch from e1300dc to 63f7ff4 Compare March 11, 2021 15:03
@maelvls maelvls requested a review from wallrj March 12, 2021 11:20
@maelvls maelvls self-assigned this Mar 12, 2021
Copy link
Member

@wallrj wallrj left a comment

Choose a reason for hiding this comment

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

Thanks @maelvls

Just a few comments and nits, but I know that this has already been deployed, so I'll add lgtm and hold so that you can decide when it gets merged.

/lgtm
/hold

Comment on lines +107 to +109
**Note:** the preflight deploymnent is expected to be failing when the
application is first deployed. After registering your cluster on
<https://platform.jetstack.io>, the deployment will start working. To register your cluster, keep reading the [next section](#step-2-log-into-the-jetstack-secure-dashboard).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
**Note:** the preflight deploymnent is expected to be failing when the
application is first deployed. After registering your cluster on
<https://platform.jetstack.io>, the deployment will start working. To register your cluster, keep reading the [next section](#step-2-log-into-the-jetstack-secure-dashboard).
**Note:** the preflight deployment is expected to be failing when the
application is first deployed. After registering your cluster on
<https://platform.jetstack.io>, the deployment will start working. To register your cluster, keep reading the [next section](#step-2-log-into-the-jetstack-secure-dashboard).

@@ -185,8 +189,8 @@ gcloud container clusters get-credentials --zone=$LOCATION $CLUSTER
You can then apply the Jetstack Secure agent configuration to your cluster:

```sh
kubectl -n $NAMESPACE apply -f agent-config.yaml
kubectl -n $NAMESPACE rollout restart deploy jetstack-secure-preflight
cat agent-config.yaml | sed '/namespace:/d' | kubectl -n $NAMESPACE apply -f-
Copy link
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
cat agent-config.yaml | sed '/namespace:/d' | kubectl -n $NAMESPACE apply -f-
sed '/namespace:/d' agent-config.yaml | kubectl -n $NAMESPACE apply -f-

kubectl -n $NAMESPACE apply -f agent-config.yaml
kubectl -n $NAMESPACE rollout restart deploy jetstack-secure-preflight
cat agent-config.yaml | sed '/namespace:/d' | kubectl -n $NAMESPACE apply -f-
kubectl -n $NAMESPACE rollout restart $(kubectl -n $NAMESPACE get deploy -oname | grep preflight)
Copy link
Member

Choose a reason for hiding this comment

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

nit I thought it might be possible to use kubectl rollout restart deployment --selector app.kubernetes.io/name=preflight but it doesn't support the --selector flag.

The kubectl scale command does.

Did we discuss perhaps deploying preflight with replicas=0 ? I can't remember why we decided against it.

Copy link
Member Author

@maelvls maelvls Mar 12, 2021

Choose a reason for hiding this comment

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

We discussed having two separate behaviours: one for the tests, one for the normal deployments.

  • When running the smoke-test, we set replica=0
  • When running normally (e.g., one click deployment through the marketplace UI), the preflight deployment would be set to replica=1 (= purposefully failing)

I don't really like the idea of letting the preflight deployment in a failing mode "by default", e.g. the user would immediately see a failing deployment after deploying:

The test-1 application page on GKE should show the test-1 application. The preflight deployment is failing because the user has not (yet) gone to http://platform.jetstack.io/ to register their cluster. This screenshot is stored in this issue: https://github.com/jetstack/jetstack-secure-gcm/issues/21

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll open an issue about this.

Comment on lines +236 to +238
docker run -it --rm -v "$(pwd)":/tmp frapsoft/openssl genrsa -out /tmp/ca.key 2048
docker run -it --rm -v "$(pwd)":/tmp frapsoft/openssl req -x509 -new -nodes -key /tmp/ca.key -subj "/CN=example" -reqexts v3_req -extensions v3_ca -out /tmp/ca.crt
kubectl create secret tls example-ca-key-pair --cert=ca.crt --key=ca.key
Copy link
Member

Choose a reason for hiding this comment

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

I think it's possible to use a cert-manager self-signed Issuer to create the CA cert and then reference that from another CA Issuer.

That might be neater than using openssl from a non-jetstack Docker image.

Copy link
Member Author

Choose a reason for hiding this comment

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

Initially I tried the SelfSignedIssuer but I could not figure it out... I guess I should double down on that 😅

If that's OK, I will /unhold and do this change in another PR
I'll add that to my todo


## Prerequisites

- Kubernetes 1.11+
Copy link
Member

Choose a reason for hiding this comment

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

We should update this, in cert-manager repo.

@@ -9,17 +9,25 @@ metadata:
app.kubernetes.io/name: "{{ .Release.Name }}"
annotations:
marketplace.cloud.google.com/deploy-info: '{"partner_id": "partner", "product_id": "jetstack-secure", "partner_name": "Jetstack"}'
kubernetes-engine.cloud.google.com/icon: >-
data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAcoAAAH0CAMAAACKMp0zAAAAM1BMVEUAAABjJtljJtljJtljJtljJtljJtljJtljJtljJtljJtljJtljJtljJtljJtljJtljJtlIWwlwAAAAEHRSTlMAENDAYPBAgDCg4CCQcLBQyVb+8QAADOtJREFUeNrs3IGSoyAQBNBBYEZEkP//2jvOylV2s7uJmxhh7PcLXd0qWpIevHJyJdgrWa44XhEca7xkZqvyNFtdEh4J9sTMSWS21k7lDby1dhZJ6O3LurcWrxxsrSz6upH5V79aviZNta2J2RB828Cl4QC/2WARh6ZexJrg8Qv6nFrUhSOdU2QnoacOPja+WdJpamo0Rng7vUGc3paOLGKHciqDnUVTSUeWWXsNfzZ1n2jkdPIMbxPtbnXZST7Zlm5ZXXEdnCCZWkRf4C5v59RoRSNLQIib2dDS5tYmdv6If7QaqKFDscOcvnZy6f3iIhk3pzuYsiyR3oQd9nRvuxfUcAp4xnibYZ8rqGEM6iGm/Mo8OQWkeKiaJz1rmbGojRjm5YkcA5402pId/YIRrGqD/BxpmxgKNCrETY0s0LDZ0IMWTGvjvKNHmFygedbQXSMq2QXPdIcr0AmhH+HGtSMBSaqRDZLUYjBIUotMX5oLdCfg3lWNRDfGAl1i+sTgZKBT3uBCqUWmD7hAtxa6hnnt2IeJxfvJrgn9Z/ANT9d8RCm1CCilFt7gnEcLoRW+Wu7ehCM7NRYc9GgRcDyghae/YgEFRiJKBRQQIsIXzCpYXCq18ESmgAoRbyq1YJzaaSF4K6IFolQj4JN0LSzhJ2dKIEo1EKUaiFINRKkGovzDHh3IAAAAAAzyt75HewoxrmRcybiScSXjSsaVjCsZVzKuZFzJuJJxJeNKxpWMKxlXMq5kXMm4knEl40rGlYwrGVcyrmRcybiScSXjSsaVjCsZVzKuZFzJuJJxJeNKxpWMKxlXMq5kXMm4knEl40rGlYwrGVcyrmRcybiScSXjSsaVjCsZVzKuZFzJuJJxJeNKxpWMKxlXMq5kXMmIXXvRTRyGgjB8gm+5Er//0269u61EoSK2oJlM5n8BS/MJpDgRJU2ipEmUNImSJlHSJEqaREmTKGkSJU2ipEmUNImSJlHSJEqaREmTKGkSJU2ipEmUNImSJlHSJEqaREmTKGkSJU2ipEmUP+bmtEafhjEfI1E+7rraV1M6hKYoH0JOdpsPGT5R3hdWu6sbMnqivOvS2aN8Bk+UmyRLEfxPVpQPJY/4uxTlRslSysiJcrukYW8lygpJ6zNwotwsWUJ+JBFljaTFjJsot0uWgK/wRFklaXOG7QCUbk4xrmm+5Jt2kbQ1w4ZO6Xxnn3XLmL/aR9KmDBs2pevttviJuYtkKcOGTBnW6huXdklRvrHLVH2r3S4pyvba9+3D608SZUUt+1Zatp8kytt+b1//tpOOfN0DSvlk3+tekrZk2DApn+07hTeedNTXXJCUz/dNLz/p8DcEkJQb9p3eedJBvyMApNy073UXyQ75Sy08ym37Li8/6eDvRQApN+7bv+okkicRQMrN++4gOSH/vcJRbt93/HXJ7pKhw6Ks2NdJEpmyZl8nSWDKqn0vkryjvGaQ6vaV5B0lzAVG3b6TJGEpK/f1kkSlrN13kCQoZe2+XZDk99JzynFY4kc+uVzRe/f1kqymDPNkX3V+zBW9cd9RkpWUIXV221/M3SWTJB9RLnVfonbz/pK9JB9SxvxTgz3M7y3ZjZKsoxwAv138ty/6SfvkLdZJlrwkAYsWKyVLSZJ4RetrJUsXScIVzRokLUoSrmhWLVlykkTLzFokbZUkWmYWqiVLQZJgmZlrkbRBkliFQtkiaV6SWLlC2SJpUZJYFcrUImmdJLH6TjnY5iSJVTKzpUnSJIlVoYxNkiZJrJZC2SQZJYlVNLOpRdK8JLEqlNYiaQP6vieTzJ191CJpI/i+Z5PMVnINkhF839NJhk/KwSq7Yu97OsnsrDTXS0bsfc8n+Z8yDVabg973hJI5WWmy2hbofc8oWShbitD7nlIyR2upD8j7nlMyR0mSSGZJskgGSZJIZidJEsl8lSSJZE6SzCR5SbIUJcmSJFkKkmTJSZKlWZIsLZJkKUqSpU6SJI2SZMlJkqUkSZZWSbLUS5IlSbLkJMnSLEmWvCRZ6iVJUpAkS06SLCVJshQlyVInSZIukmRpkCRLXpIsTZIkaZQkS4MkWfKSZOkPe/eCnCoQRGH48BgQRmD2v9p7SaIiFXmkAp7unH8BUtVfQWAGTZSkk1pJeqnBd0VJ2qvEtzHPV5K7FmB73vlKct8CbEc7X0nufEMrss5Xkrv3KlvO+Upy/2s9V8r5SvIHP6I1MM5Xkj/a4CKcryQXinhZTzffMyXbvv7ftUhWyvG6km2+5x2p7SJuVU1IFqrxukg239OOlFezj6ktYF6wUM4039OOFLpvPqlP7LVYquOZ73lHai8vZkHeFUtFmvkefqT145SJuwqL5STzJZBktwxY7soxXwrJcRrENVjuQjFfEkmAebVhwEotwXxpJFEl2gLWur5/vjySQJNYa7BWfPt8mSSJT8sBq+WSnNYmzgLW6yRp4Sb2ivWiJC1cYS/YUC7JSVmirMWWSklOS5TV22YjyWmcu10Rm+olyb7gk2NbgyTZL7AlNhYkSU6ZYWNXSd67JMIabC1KknvPssLmCkneYnzFp8X2SkkyrxDU2F4WJEl8fY3YUSNJ3o2RHnuKkvyoToRV2FUhSdYnkRb7KiXJ+p2wDjuTJKlkyrAzSZJKNpglSaOS6YLnJGlVssBzkrQqOd/ekqRZyRZPSdKs5OxJRJJ2JUOGSZK0K/n8IrMkDUumiEeStCzZ4JEkLUumiHuSNC3Z454kTUumCrckaVuywC1J2pZ8nJSSNC5Z4CtJGpe8n5SStC5Z4DNJWpe8nZSSNC9Z4CNJmpdMFcYkaV+ywJgk7Ut+npSSdCBZAIAkHUimCoAkPUg2ACTpQTJFQJIuJBtAki4kQyZJH5KplqQTyZBJ0odkKiXpRDLHbxcluT3y1YGGecJ+JXv8flngnbBfyRRxQB3thB1L1jiklnTCjiXbDIdUcU7YsWQacFA944Q9SxY4qhj4JuxZMkUcVkc3YdeSNQ4sJ5uwa8k2w4FVXBN2LZkGHNqVacK+JXscWxZ4JuxbMkQc3EAzYd+SqcPh9SQTdi5Z4PhioJiwc8l0wQl1DBP2LlnjlIr3T9i7ZI5ziiFJcp7By+tYJ8l5Ji+vY4Ukn7N5eR2LQZJPGb28jnWSnGb28jpWSPKR3cvrWBYkeUzhgpMbJDnJ1trrvEaSX5lbe52XtZK8R/y1rS1Vkrxl5s2BV9WSvGXin08sVUjyd8szvKkYJGn7OeTRIMlk+znkUSlJO6/YrSRJI28wQ5IrWd0PkeQsN38oJZmof/xskiRXcvFEKcmUvDxRSvJ/xL+YNU2S69lfepXkmPE9Skk++qNrA5L0cssjSS+3PJL0cssjSS/bIZL0ssojyaVChJkk6WVjS5Jebl4l6eXmVZJetigl6eXmVZKS/BOShlZeJfmvvTvLbhQGAigqQGIQhtT+V9vtjo9DHMBMaUrFewvwh+8RSEI2SF5CUj5cKiFpZUGJpKr/ANlTh6SRBaVzDZJGJJ1rkDRxKOtegWTyWwPPbxpJG5L371qrZCjuyZKQfHzbGiV737rP6m7ph6T+tzxHWKqTrFo3rG3kfdfcrnspi7oki9a9Vs99FJJf1YreFSTSrVs2IfliqUfSu/G8jIfkq6V2yaltRiR/WmqXHL/GIjlmqV1ybHqG5Kjl6W+YEfEr37CK5JSlcskfO8ZITlsql3wZlkjOWOa6Jb+/jxzJeQXdkq6SZ0i+c1At+YtX2GhM8i6hWdI5+YqnWm8tNEu6KPeQXKbRK5b8XI4gubRGr+TvUPZWJZ1r1Eo+KS98SnJdlVZJF0WQXJVXKunkXxf9NcHGdEqW8uiCv/CZKk3Jw3d7ApInSboowmbdt1KVLOXQciTPkHwuRdgYGJSsZClH1iB5mmQWheXksGQlXSPCImRQupKdHFcoHZ0m6UV4zjwISaauliSZ8FiRTOsNlOMhyYTHkCR7dVYkDR/9uJgkj5mNSIZ0/hJ0JiS5TdqR5EGIFUlWk0Yk2XQdLz3JgovreF1qkqxBJqtjSpJs1c2V9elIcnF9UxcSkWTm+rY6T0GSmeuSspt+SXbPF/YRdEvyg5DlZYVmyTyhd20rqNMryWJyZXWuUzKymFzfTaPkjfnOlsqoTZJnzFvLGl2SLEF2LUv0SDIk95X1WiQZkgcMTA2SDMljBub5kgzJgyrjPkmGpJ6y2x5JjtSpqoybJdne0VZ1jmTFkDy+ttgiycFzldX/WTJw6ENNnhWIkTzTHSPtkQw8YFbUHsmeMx+K8lxbjeSZtxrJc+TDSJslC26SuvLcJI3kOXRuJL8Nko1zdXlmO0baJNkw29GXB9JIGyQLpq0a80AayQNppYZ7pJnaW5CFBSCVl1WBDQEr/cUE0ky+kJlyD2RCtd2EZtFxi0yvsupzGRT7qmQ8pltWPjI/GP8AjZ79v4HwnFEAAAAASUVORK5CYII=
Copy link
Member

Choose a reason for hiding this comment

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

Ah, that's how it's done!

Copy link
Member Author

Choose a reason for hiding this comment

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

had to dig into click-to-deploy to find this piece of the Application API 😅

@@ -48,6 +48,7 @@ Selector labels
{{- define "preflight.selectorLabels" -}}
app.kubernetes.io/name: {{ include "preflight.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
app.kubernetes.io/component: preflight
Copy link
Member

Choose a reason for hiding this comment

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

nit. Are we calling it preflight, or are we calling it jetstack-secure-agent, or somthing else?
I guess preflight makes sense until we change the name of the chart.

Copy link
Member Author

@maelvls maelvls Mar 12, 2021

Choose a reason for hiding this comment

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

I am quite confused by which name I should be employing: in places, I call it the "Jetstack Secure Platform agent", and sometimes I default to "the preflight agent".

@charlieegan3 thoughts?

app.kubernetes.io/instance: "{{ .Release.Name | trunc 63 | trimSuffix "-" }}"
app.kubernetes.io/component: ubbagent
Copy link
Member

Choose a reason for hiding this comment

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

Can I wondered if we could use more descriptive values for these component names, e.g. Google Cloud Marketplace Billing Agent....

BUT WAIT why is this still here as a separate deployment, if you have now deployed the ubbagent as a cert-manager side-car?

Copy link
Member

Choose a reason for hiding this comment

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

...ah, this isn't the deployment, but the config for ubbagent...ignore me.
I see the deployment is deleted below.

Copy link
Member Author

@maelvls maelvls Mar 12, 2021

Choose a reason for hiding this comment

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

Yes, I moved it to the cert-manager deployment itself instead of having it qs q separate deployment. Apologies for the messy PR :-(

@maelvls
Copy link
Member Author

maelvls commented Mar 12, 2021

/unhold

I created #40 with the PR comment fixes.

@jetstack-bot jetstack-bot merged commit f52b982 into main Mar 12, 2021
@maelvls maelvls deleted the use-jetstack-public-to-build branch March 12, 2021 15:59
maelvls added a commit that referenced this pull request Mar 13, 2021
Signed-off-by: Maël Valais <[email protected]>
Co-authored-by: Richard Wall <[email protected]>
maelvls added a commit that referenced this pull request Mar 16, 2021
As Richard pointed out, there is no need to use openssl (and a Docker
image of openssl that is not jetstack-branded) when we can simply use a
self-signed issuer for the same purpose.

Signed-off-by: Maël Valais <[email protected]>
Co-authored-by: Richard Wall <[email protected]>
maelvls added a commit that referenced this pull request Mar 24, 2021
Fixes for the PR comments in #35
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.

3 participants