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

fix(argo-cd): Update CONTRIBUTING docs #1782

Merged
merged 11 commits into from
Jan 20, 2023
125 changes: 74 additions & 51 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,87 @@

Argo Helm is a collection of **community maintained** charts. Therefore we rely on you to test your changes sufficiently.


# Pull Requests
## Pull Requests

All submissions, including submissions by project members, require review. We use GitHub pull requests for this purpose. Consult [GitHub Help](https://help.github.com/articles/about-pull-requests/) for more information on using pull requests. See the above stated requirements for PR on this project.

### Pull Request Title Linting

We lint the title of your pull request to ensure it follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) specification. This is done using GitHub actions and the [action-semantic-pull-request](.github/workflows/pr-title.yml) workflow. We require the scope of the change to be included in the title. The scope should be the name of the chart you are changing. For example, if you are changing the `argo-cd` chart, the title of your pull request should be `fix(argo-cd): Fix typo in values.yaml`.

## Documentation

The documentation for each chart is generated with [helm-docs](https://github.com/norwoodj/helm-docs). This way we can ensure that values are consistent with the chart documentation.

We have a script on the repository which will execute the helm-docs docker container, so that you don't have to worry about downloading the binary etc. Simply execute the script (Bash compatible, might require sudo privileges):

```shell
./scripts/helm-docs.sh
```

> **Note**
> When creating your own `README.md.gotmpl`, don't forget to add it to your `.helmignore` file.

### Updating a chart README.md

When updating the `README.md.gotmpl` inside a chart directory you must to run the `helm-docs` script to generate the updated `README.md` file. To reiterate, you should not edit the `README.md` file manually. It will be generated by the following command:

```shell
./scripts/helm-docs.sh
```

> **Note**
> If you see changes to unrelated chart `README.md` files you may have accidentally updated a `README.md.gotmpl` file in another chart's folder unintentionally or someone else failed to run this script. Please revert those changes if you do not intend them to be a part of your pull request.

## Versioning

Each chart's version follows the [semver standard](https://semver.org/). New charts should start at version `1.0.0`, if it's considered stable. If it's not considered stable, it must be released as [prerelease](#prerelease).
Each chart's version follows the [semver standard](https://semver.org/).

New charts should start at version `1.0.0`, if it's considered stable. If it isn't considered stable, it must be released as `prerelease`.

Any breaking changes to a chart (backwards incompatible) require:

* Bump of the current Major version of the chart
* State possible manual changes for this chart version in the `Upgrading` section of the chart's `README.md.gotmpl` ([See Upgrade](#upgrades))
* Bump of the current Major version of the chart
* State possible manual changes for this chart version in the `Upgrading` section of the chart's `README.md.gotmpl`

### New Application Versions

When selecting new application versions ensure you make the following changes:

* `values.yaml`: Bump all instances of the container image version
* `Chart.yaml`: Ensure `appVersion` matches the above container image and bump `version`

Please ensure chart version changes adhere to semantic versioning standards:

* Major: Large chart rewrites, major non-backwards compatible or destructive changes
* Minor: New chart functionality (sidecars), major application updates or minor non-backwards compatible changes
* Patch: App version patch updates, backwards compatible optional chart features

### Immutability

Each release for each chart must be immutable. Any change to a chart (even just documentation) requires a version bump. Trying to release the same version twice will result in an error.

### Chart Versioning

Currently we require a chart version bump for every change to a chart, including updating information for older verions. This may change in the future.

### Artifact Hub Annotations

Since we release our charts on Artifact Hub we encourage making use of the provided chart annotations for Artifact Hub.

* [https://artifacthub.io/docs/topics/annotations/helm/](https://artifacthub.io/docs/topics/annotations/helm/)
* [https://artifacthub.io/docs/topics/annotations/helm/](https://artifacthub.io/docs/topics/annotations/helm/)

#### Changelog

We want to deliver transparent chart releases for our chart consumers. Therefore we require a changelog per new chart release.

Changes on a chart must be documented in a chart specific changelog in the `Chart.yaml` [Annotation Section](https://helm.sh/docs/topics/charts/#the-chartyaml-file). For every new release the entire `artifacthub.io/changes` needs to be rewritten. Each change requires a new bullet point following the pattern `- "[{type}]: {description}"`. You can use the following template:
Changes on a chart must be documented in a chart specific changelog in the `Chart.yaml` [Annotation Section](https://helm.sh/docs/topics/charts/#the-chartyaml-file).

```
A new `artifacthub.io/changes` needs to be written covering only the changes since the previous release.

Each change requires a new bullet point following the pattern `- "[{type}]: {description}"`. You can use the following template:

```yaml
name: argo-cd
version: 3.4.1
...
Expand All @@ -45,67 +94,54 @@ annotations:
- "[Deprecated]: Something deprecated"
- "[Removed]: Something was removed"
- "[Fixed]: Something was fixed"
- "[Security]": Some Security Patch was included"
- "[Security]: Some Security Patch was included"
```

## Documentation

The documentation for each chart is done with [helm-docs](https://github.com/norwoodj/helm-docs). This way we can ensure that values are consistent with the chart documentation.
## Testing

We have a script on the repository which will execute the helm-docs docker container, so that you don't have to worry about downloading the binary etc. Simply execute the script (Bash compatible, might require sudo privileges):

```
bash scripts/helm-docs.sh
```

**NOTE**: When creating your own `README.md.gotmpl`, don't forget to add it to your `.helmignore` file.



# Testing

## Testing Argo Workflows Changes
### Testing Argo Workflows Changes

Minimally:

```
```shell
helm install charts/argo-workflows -n argo
argo version
```

Follow this instructions for running a hello world workflow.

## Testing Argo CD Changes
### Testing Argo CD Changes

Clean-up:

```
```shell
helm delete argo-cd --purge
kubectl delete crd -l app.kubernetes.io/part-of=argocd
```

Pre-requisites:
```

```shell
helm repo add redis-ha https://dandydeveloper.github.io/charts/
helm dependency update
```

Minimally:

```
```shell
helm install argocd argo/argo-cd -n argocd --create-namespace
kubectl port-forward service/argo-cd-argocd-server -n argocd 8080:443
```

In a new terminal:

```
```shell
argocd version --server localhost:8080 --insecure
# reset password to 'Password1!'
kubectl -n argocd patch secret argocd-secret \
-p '{"stringData": {
"admin.password": "$2a$10$hDj12Tw9xVmvybSahN1Y0.f9DZixxN8oybyA32Uy/eqWklFU4Mo8O",
"admin.passwordMtime": "'$(date +%FT%T%Z)'"
"admin.password": "$2a$10$hDj12Tw9xVmvybSahN1Y0.f9DZixxN8oybyA32Uy/eqWklFU4Mo8O",
"admin.passwordMtime": "'$(date +%FT%T%Z)'"
}}'
argocd login localhost:8080 --username admin --password 'Password1!'

Expand All @@ -114,38 +150,25 @@ argocd login localhost:8080 --username admin --password 'Password1!'

Create and sync app:

```
```shell
argocd app create guestbook --dest-namespace default --dest-server https://kubernetes.default.svc --path guestbook --project default --repo https://github.com/argoproj/argocd-example-apps.git
argocd app sync guestbook
```

## New Application Versions

When raising application versions ensure you make the following changes:

- `values.yaml`: Bump all instances of the container image version
- `Chart.yaml`: Ensure `appVersion` matches the above container image and bump `version`

Please ensure chart version changes adhere to semantic versioning standards:

- Patch: App version patch updates, backwards compatible optional chart features
- Minor: New chart functionality (sidecars), major application updates or minor non-backwards compatible changes
- Major: Large chart rewrites, major non-backwards compatible or destructive changes

## Testing Charts
### Testing Charts

As part of the Continuous Integration system we run Helm's [Chart Testing](https://github.com/helm/chart-testing) tool.

The checks for this tool are stricter than the standard Helm requirements, where fields normally considered optional like `maintainer` are required in the standard spec and must be valid GitHub usernames.
The checks for Chart Testing are stricter than the standard Helm requirements. For example, fields normally considered optional like `maintainer` are required in the standard spec and must be valid GitHub usernames.

Linting configuration can be found in [ct-lint.yaml](./.github/configs/ct-lint.yaml)

The linting can be invoked manually with the following command:

```
```shell
./scripts/lint.sh
```

## Publishing Changes

Changes are automatically publish whenever a commit is merged to main. The CI job (see `./.github/workflows/publish.yml`).
Changes are automatically publish whenever a commit is merged to the `main` branch by the CI job (see `./.github/workflows/publish.yml`).
4 changes: 2 additions & 2 deletions charts/argo-cd/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ appVersion: v2.5.7
kubeVersion: ">=1.22.0-0"
description: A Helm chart for Argo CD, a declarative, GitOps continuous delivery tool for Kubernetes.
name: argo-cd
version: 5.18.0
version: 5.18.1
home: https://github.com/argoproj/argo-helm
icon: https://argo-cd.readthedocs.io/en/stable/assets/logo.png
sources:
Expand All @@ -23,4 +23,4 @@ dependencies:
condition: redis-ha.enabled
annotations:
artifacthub.io/changes: |
- "[Added]: Extra secret labels with .Values.configs.secret.labels"
- "[Fixed]: README information about 5.12.0 TLS changes"
5 changes: 3 additions & 2 deletions charts/argo-cd/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ This version reduces history limit for Argo CD deployment replicas to 3 to provi

### 5.12.0

This version deprecates the `configs.secret.argocdServerTlsConfig` option. Use `server.certificate` or `server.certificateSecret` to provide custom TLS configuration for Argo CD server.
If you terminate TLS on ingress please use `argocd-server-tls` secret instead of `argocd-secret` secret.
If Argo CD is managing termination of TLS and you are using `configs.secret.argocdServerTlsConfig` option to provide custom TLS configuration for this chart, please use `server.certificate` or `server.certificateSecret` instead.
For the secrets for tls termination, please use a secret named `argocd-server-tls` instead of `argocd-secret`.
For the technical details please check the [Argo CD documentation](https://argo-cd.readthedocs.io/en/stable/operator-manual/tls/#tls-certificates-used-by-argocd-server). When transitioning from the one secret to the other pay attention to `tls.key` and `tls.crt` keys.

### 5.10.0

Expand Down
5 changes: 3 additions & 2 deletions charts/argo-cd/README.md.gotmpl
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ This version reduces history limit for Argo CD deployment replicas to 3 to provi

### 5.12.0

This version deprecates the `configs.secret.argocdServerTlsConfig` option. Use `server.certificate` or `server.certificateSecret` to provide custom TLS configuration for Argo CD server.
If you terminate TLS on ingress please use `argocd-server-tls` secret instead of `argocd-secret` secret.
If Argo CD is managing termination of TLS and you are using `configs.secret.argocdServerTlsConfig` option to provide custom TLS configuration for this chart, please use `server.certificate` or `server.certificateSecret` instead.
For the secrets for tls termination, please use a secret named `argocd-server-tls` instead of `argocd-secret`.
For the technical details please check the [Argo CD documentation](https://argo-cd.readthedocs.io/en/stable/operator-manual/tls/#tls-certificates-used-by-argocd-server). When transitioning from the one secret to the other pay attention to `tls.key` and `tls.crt` keys.

### 5.10.0

Expand Down
6 changes: 3 additions & 3 deletions scripts/helm-docs.sh
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
#!/bin/bash
## Reference: https://github.com/norwoodj/helm-docs
set -eux
CHART_DIR="$(cd "$(dirname "$0")/.." && pwd)"
echo "$CHART_DIR"
REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
echo "$REPO_ROOT"

echo "Running Helm-Docs"
docker run \
-v "$CHART_DIR:/helm-docs" \
-v "$REPO_ROOT:/helm-docs" \
-u $(id -u) \
jnorwood/helm-docs:v1.9.1
jmeridth marked this conversation as resolved.
Show resolved Hide resolved