brigadecore / charts Goto Github PK

I tried to use the 'namespace' feature, but it did not seem to work. The feature where builds get deployed into the specified namespace.
I researched why and it seems that namespace field in the project's values.yaml file is ignored.
Specifically, the helm template which constructs the project secret does not use the namespace value in the project's values.yaml.
The only way that I can determine to use this feature is to create a custom gateway that sets the namespace on the store object. I have not verified this yet though.
Consequently, this line of code in kube/project.go - NewProjectFromSecret
proj.Kubernetes.Namespace = def(secret.Data["namespace"], namespace)
can never use the value from the secret because it's never there.

As of Kubernetes version v1.16.0, there are a handful of API deprecations which require Brigade chart updates if they are to be installed on a cluster of the aforementioned version or greater.

In order to maintain compatibility for older clusters (it looks like < 1.14.x) not serving the updated API versions, we can add some templating logic similar to the example seen here.

AC:

• All charts are updated such that they install properly on K8s clusters >= v1.16.x
• All charts continue to install properly on K8s clusters < v1.16.x (Pay special attention to < v1.14.x)

Most of the charts in this repo use images tagged with latest or do not specify a tag-- which, to Docker, implies latest.

This is not a good practice as it creates the possibility that, depending on the image pull policy, a newer version of an image that was perhaps not desired can be pulled and used. Even worse, the newer image in question could be incompatible with the chart. Worse still, different versions of the image could exist on different nodes in the cluster...

Going forward, could I propose that we only use images with immutable semver tags?

Currently, brigade-github-app and kashti each have their own discrete charts and both of these are used as subcharts of the brigade chart.

I'm beginning to question whether this is sensible. As far as I am able to discern, there is no reason to ever install either Kashti or the GitHub gateway without Brigade itself. If that is the case, what is gained by these components being independently installable?

The big benefit of collapsing these two subcharts into the main chart is a simplified release process.

For most releases of Brigade, we release new versions of Kashti and the GitHub gateway simultaneously. This is the process we end up following to update and release all corresponding charts:

1. PR to rev Kashti app version in kashti chart
2. Apply tag to kick of release of kashti chart
3. PR to rev GH gateway version in brigade-github-app chart
4. Apply tag to kick of release of brigade-github-app chart
5. PR to rev Brigade app version and rev versions of kashti and brigade-github-app charts that main brigade chart depends on
6. Apply tag to kick of release of main brigade chart

Considering that PRs require reviews-- this can take longer than should.

If we collapse the two subcharts into the main chart, the process is streamlined to just this:

1. PR to rev Brigade, Kashti, and GH gateway app versions
2. Apply tag to kick of release of brigade chart

The shorter, simpler process is also less prone to human error.

I anticipate that this is probably a breaking change (i.e. I don't know how Helm would deal with this on helm upgrade quite well enough to claim it wouldn't be), so I'm going to propose this as a change for the 2.0 time frame.

A convention I have frequently observed in values.yaml files (or in YAML files in general) is that when you have explanatory comments positioned above commented YAML (such as an optional setting with no default value), ## is used for the free form text while # is used for the YAML. This convention visually distinguishes things that can be safely uncommented from things than cannot.

Example:

# Set a defaultSharedSecret if you want to use this one instance of the
# Github gateway with multiple Brigade projects. Leave the shared secret field
# blank in project-level configuration and this default will be used.
# defaultSharedSecret:

vs

## Set a defaultSharedSecret if you want to use this one instance of the
## Github gateway with multiple Brigade projects. Leave the shared secret field
## blank in project-level configuration and this default will be used.
# defaultSharedSecret:

Here's a values.yaml from kubernetes/charts chosen at random to demonstrate that this is an existing convention:

https://github.com/helm/charts/blob/master/stable/wordpress/values.yaml

batch/v1beta1 CronJob is deprecated in v1.21+, unavailable in v1.25+; use batch/v1 CronJob

Setting vacuum.successfulJobsHistoryLimit to 0 upon install/upgrade has no effect, but setting it to any positive value has the intended effect.

These lines appear in the template for the vacuum CronJob:

spec:
  # ...
  successfulJobsHistoryLimit: {{ default 10 .Values.vacuum.successfulJobsHistoryLimit }}
  failedJobsHistoryLimit: {{ default 10 .Values.vacuum.failedJobsHistoryLimit }}

Per sprig documentation, for numeric types, the default function considers 0 as unset-- hence, explicitly providing the value 0 results in getting the default-- 10.

I propose adding defaults to values.yaml and removing use of the default function here.

Are there any downsides if the vacuum would also delete other completed vacuum pods?

For my installation at least, with AWS Load Balancer, this line requires a star

path: /*

I see the ingress for the Brigade API server already supports a setting from values.yaml. Kashti needs that too.

Hey all 👋

Currently, the brigade chart doesn't allow setting custom annotations on the generic gateway service (and maybe other services as well). The general use case I see for this is for those of us who are running ambassador on our clusters and want to handle ingress using it.

It would be a nice QoL addition for this to be a configuration option for the chart rather than an extra manual step via kubectl annotate or similar.

Some of the endpoints that are intended to be exposed (such as gateways) lack an ingress resource.

This issue tracks the addition of ingress templates for all gateways.

cc @dgkanatsios for the generic gateway.

We currently lint charts via test, but it would be great to also helm install --dry-run --debug $(CHART) for each/all charts.

When using the chart, the key for brigade-github-app is ignored completely making the chart unusable.

To reproduce:

helm template brigade/brigade --name-template brigade --namespace brigade --version 1.6.0 --values values.yaml

---
brigade-github-app:
  enabled: true
  key: |
    -----BEGIN RSA PRIVATE KEY-----
    REDACTED KEY
    -----END RSA PRIVATE KEY-----

Helm version: version.BuildInfo{Version:"v3.0.0-beta.3", GitCommit:"5cb923eecbe80d1ad76399aee234717c11931d9a", GitTreeState:"clean", GoVersion:"go1.12.5"}

Doing a fresh install of Brigade, I did the following:

$ helm repo add brigade https://brigadecore.github.io/charts
$ helm repo list
NAME   	URL
brigade	https://brigadecore.github.io/charts
stable 	https://kubernetes-charts.storage.googleapis.com/
$ helm install brigade brigade/brigade
NAME: brigade
LAST DEPLOYED: 2019-11-04 16:38:12.298867 -0700 MST m=+1.685438437
NAMESPACE: default
STATUS: deployed
NOTES:
Brigade is now installed!

To find out about your newly configured system, run:

  $ helm status brigade

NOTE: The Helm repo URL serving this chart has changed to: https://brigadecore.github.io/charts

All subsequent chart releases will be served at this URL.

Please update your local Helm repo:

  helm repo remove brigade
  helm repo add brigade https://brigadecore.github.io/charts

The NOTE at the bottom seems spurious. The URL it recommends is the URL I used.

Wondering if it'd be worth refactoring charts/brigade to use a top-level/global image config, primarily for setting image.repository and image.tag to be used by all deployments. (Currently, each component's image config needs changing when non-defaults are desired.)

(Migrated from Azure/brigade-charts#3)

The new "sshCert" key is needed in a brigade project Secret to run jobs when using brigade 1.3.0.

brigadecore/brigade#1008

But, brigade-project chart currently doesn't support it.
Do you have any plans to support it?

See brigadecore/brigade-github-app#65 as an example.

To consume brigade and also add exceptions for certain OPA Policies that we apply, it would be beneficial if it was possible to pass on annotations to the deployment.

@vdice let me know what you think

It seems like some charts are missing the resource requests and limits on pods. We should add those and making them configurable.

In both cases, these components attempt to list pods, but the roles they are bound to do not have that permission.

Please make the livenessProbe settings such as:

periodSeconds
initialDelaySeconds
timeoutSeconds

configurable through helm values.

Use https://github.com/helm/chart-testing to test prs and updates to charts.

This is a very simple tool that can automatically test charts when they are updated via a pr.

This includes checking for valid authors, requiring version bumps for all changes (like the official helm repo) and linting the charts.

How to use:

test/ci.yaml

remote: origin
target-branch: master
chart-dirs:
  - charts
excluded-charts: []
chart-repos: []
helm-extra-args: --timeout 600

Commands to run:

ct version
ct lint --config test/ct.yaml

For output see any pr in the official helm repo i.e for this pr helm/charts#10151 the output is https://circleci.com/gh/helm/charts/45660?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Currently, the brigade-github-app chart creates a config map that includes a sensitive key.pem field. The deployment mounts this config map to /etc/brigade-github-app and, in turn, the containerized brigade-github-app process in each pod consumes /etc/brigade-github-app/key.pem.

Since the contents of key.pem are sensitive, it would be more proper and more safe to store this in a secret.

This can be accomplished pretty easily with changes to the chart and no changes to the brigade-github-app software itself.

This is not a breaking change:

1. Eliminate the brigade-github-app config map (since key.pem) is its only field anyway.

2. Add key.pem field to the existing brigade-github-app secret.

3. In the deployment, mount the brigade-github-app secret to /etc/brigade-github-app in place of the brigade-github-app config map.

This is a small request, however having option to define a secret name for the chart instead of passing defaultSharedSecret and key values would be really nice. This is useful in case of following GitOps approach, Flux with Helm Operator and storing secrets in Git with Sealed Secrets.

brigadecore/kashti#146

Changes: https://github.com/brigadecore/kashti/pull/146/files