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

Secrets consistency for the chart. Bugfixing. #1

Closed
wants to merge 16 commits into from

Conversation

ksa-real
Copy link
Owner

@ksa-real ksa-real commented Jun 22, 2023

  • Enabling existing secrets for external MySQL and Redis
  • Tolerate existing secrets for bundled charts.
  • README.md: secrets handling explained.
  • Fixed multiple bugs where missing required field was replaced with default instead of failing.
  • PHONE_NOTIFICATIONS_LIMIT was on the wrong level: it was not set if existingSecret was true.

Next are the cosmetic changes. They improve chart consistency, e.g. prevent generation of multiple new lines in certain cases:

  • Common approach to spaces trimming. This typically allows curly blocks and actual strings indentation and nice nindent usage:

    • Two curly blocks should not trim the same space. I.e. "{{ ... -}} {{- ... }}" shouldn't happen.

    • Template generates either single line or multiline string. In both cases, no new line appears on both sides of the output string. So we delete unnecessary new lines inside and at the end of string with "trim-to-left" ({{- ) and the leading new line using "trim-to-right" (-}}). Note that trimming both leading and trailing new line is not always easily possible: Function similar to nindent but ignoring content's initial new line. Masterminds/sprig#357

      Example.

      {{- define "mytemplate" -}} {{ if someBoolean -}} {{ .Value.some }} {{- else -}} some string {{- end }} {{- end }}

  • template replaced with include. It is often recommended to use include by default, as it allows pipelining.

What this PR does

Which issue(s) this PR fixes

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • CHANGELOG.md updated (or pr:no changelog PR label added if not required)

joeyorlando and others added 11 commits June 12, 2023 15:14
get rid of need for FEATURE_PROMETHEUS_EXPORTER_ENABLED to be present in ci-test.py settings file
Merge this PR to `main` branch to start another [github actions
job](https://github.com/grafana/oncall/blob/dev/.github/workflows/helm_release.yml)
that will release the updated version of the chart (version: 1.2.42,
appVersion: v1.2.42) into `grafana/helm-charts` helm repository.

This PR was created automatically by this [github
action](https://github.com/grafana/oncall/blob/dev/.github/workflows/helm_release_pr.yml).

Co-authored-by: GitHub Actions <[email protected]>
Merge: Release oncall Helm chart 1.2.44
Co-authored-by: Michael Derynck <[email protected]>
Co-authored-by: Joey Orlando <[email protected]>
Co-authored-by: Matias Bordese <[email protected]>
Co-authored-by: Rares Mardare <[email protected]>
Co-authored-by: Joey Orlando <[email protected]>
Co-authored-by: Ildar Iskhakov <[email protected]>
Co-authored-by: Ruslan Gainanov <[email protected]>
Co-authored-by: Yulia Shanyrova <[email protected]>
# What this PR does

## Which issue(s) this PR fixes

## Checklist

- [ ] Unit, integration, and e2e (if applicable) tests updated
- [ ] Documentation added (or `pr:no public docs` PR label added if not
required)
- [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
# What this PR does

## Which issue(s) this PR fixes

## Checklist

- [ ] Unit, integration, and e2e (if applicable) tests updated
- [ ] Documentation added (or `pr:no public docs` PR label added if not
required)
- [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
[css fixes, displaying chatops button visibility fix]
@ksa-real ksa-real force-pushed the existing-secrets branch 2 times, most recently from f409dc9 to 55462d7 Compare June 22, 2023 10:41
- Enabling existing secrets for external MySQL and Redis
- Tolerate existing secrets for bundled charts.
- README.md: secrets handling explained.
- Fixed multiple bugs where missing required field was replaced with
  default instead of failing.
- PHONE_NOTIFICATIONS_LIMIT was on the wrong level: it was not set if
  existingSecret was true.

Next are the cosmetic changes. They improve chart consistency, e.g.
prevent generation of multiple new lines in certain cases:
- Common approach to spaces trimming. This typically allows curly blocks
  and actual strings indentation and nice `nindent` usage:
  - Two curly blocks should not trim the same space. I.e.
    "{{ ... -}} {{- ... }}" shouldn't happen.
  - Template generates either single line or multiline string. In both
    cases, no new line appears on both sides of the output string. So we
    delete unnecessary new lines inside and at the end of string with
    "trim-to-left" (`{{-` ) and the leading new line using
    "trim-to-right" (`-}}`).
    Note that trimming both leading and trailing new line is not
    always easily possible:
    Masterminds/sprig#357

    Example.

    ```
    {{- define "mytemplate" -}}
    {{ if someBoolean -}}
      {{ .Value.some }}
    {{- else -}}
      some string
    {{- end }}
    {{- end }}
    ```

- `template` replaced with `include`. It is often recommended to use
  `include` by default, as it allows pipelining.
@ksa-real ksa-real closed this Jun 23, 2023
@ksa-real ksa-real deleted the existing-secrets branch June 23, 2023 06:24
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.

9 participants