feature(chart): Add extraObjects field to support additional manifest…

[cloudziu]

Hello! First of all, thank you for your hard work on this project.

Description I would like to propose a feature, that will allow adding custom manifest files into the Helm Chart. I took the idea of the implementation from a different helm chart repository.

An example use case is to include the External-DNS secret file as an ExternalSecret Object.

I've tested it locally with helm template, helm install and helm uninstall. Additional manifest were added to the chart and also removed when the chard was uninstalled.

• added charts/external-dns/templates/extramanifests.yaml
• updated charts/external-dns/templates/_helpers.tpl with external-dns.render
• updated charts/external-dns/values.yaml with an example extraObjects: [] field

Example

# values.yaml
extraVolumes:
  - name: azure-config-file
    secret:
      secretName: azure-config-file

extraVolumeMounts:
  - name: azure-config-file
    mountPath: /etc/kubernetes
    readOnly: true

extraObjects:
  - apiVersion: external-secrets.io/v1beta1
    kind: ExternalSecret
    metadata:
      name: azure-config-file
      namespace: '{{ .Release.Namespace }}'
    spec:
      refreshInterval: 1h
      secretStoreRef:
        kind: ClusterSecretStore
        name: storeName
      target:
        name: azure-config-file
        creationPolicy: Owner
      data:
      - secretKey: azure.json
        remoteRef:
          key: secret/external-dns-config

Helm template output (truncated):

---
# Source: external-dns/templates/extramanifests.yaml
apiVersion: external-secrets.io/v1beta1
kind: ExternalSecret
metadata:
  name: azure-config-file
  namespace: 'external-dns'
spec:
  data:
  - remoteRef:
      key: secret/external-dns-config
    secretKey: azure.json
  refreshInterval: 1h
  secretStoreRef:
    kind: ClusterSecretStore
    name: storeName
  target:
    creationPolicy: Owner
    name: azure-config-file

Checklist

• [ ] Unit tests updated
• [ ] End user documentation updated

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: cloudziu / name: Cloudziu (53d0713a9d613965653b2b58678cd3644c2d2551, 940cf4429f611fbf85a69b265a47108f1c06f369)

[k8s-ci-robot]

Welcome @cloudziu!

It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

[k8s-ci-robot]

Hi @cloudziu. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[mloiseleur]

This PR looks technically ok. You need to update Changelog, too. Chart maintainer will decide if we should support this feature. /assign @stevehipwell

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from stevehipwell. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• charts/OWNERS

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[cloudziu]

Hey @mloiseleur as you suggested, I've updated the changelog. Thank you

[k8s-ci-robot]

@stevehipwell: Closed this PR.

In response to this:

@cloudziu thanks for the PR but we explicitly don't support creating additional unconstrained resources in the Helm chart. The External DNS Helm chart is intended to be an opinionated way to install External DNS in a Kubernetes cluster. I know other charts have support for this, but that doesn't make it a good idea.

If you require further customisation there are lots of ways to do this; you can extend Helm (e.g. post-render), wrap this Helm chart, or use native Kubernetes manifests. Passing Kubernetes resources through a Helm chart doesn't offer any benefits but it does add complexity and potentially offer a number of downsides.

Was there a specific problem you were trying to solve?

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[cloudziu]

Hey @stevehipwell thanks for your response.

Was there a specific problem you were trying to solve?

For instance the one posted in the example I provided. Where I add an externalSecret resource. I think that the benefit of this is the possibility to maintain other resources that we see nesecery to include to the chart within helm state.

But I understand your reasoning.

[mrdima]

@stevehipwell Actually I too would like to see this option, for example to add a cilium network policy manifest. We currently use dependency chart, but that and including the other mentioned options are cumbersome. Deploying using a gitops tool, keeping related things together, makes it quite easy and nice to deploy from the helm chart and therefor does add benefits in my view. Can you elaborate what complexity it adds and details on the downsides?

[stevehipwell]

@mrdima I think a better approach would be to explain to me why passing arbitrary resources through a Helm chart is better than working directly with the manifest or creating a wrapping Helm chart that understands the resource context?

FYI there wouldn't be templating support unless the resource was just being passed as a raw string as the code in the PR wouldn't have actually worked for templating non-string inputs.