pihole-kubernetes icon indicating copy to clipboard operation
pihole-kubernetes copied to clipboard

Add the ability to specify extra objects (manifests)

Open fernferret opened this issue 2 years ago • 4 comments

I've seen this trend in other helm charts like ArgoCD and I really like being able to bundle my "extra" bits that shouldn't be part of the chart with the template. This allows tracking them in the same location so they never get forgotten and they're tied to the release (when it gets uninstalled)

In my specific chart I added:

extraObjects:
  - apiVersion: cert-manager.io/v1
    kind: Certificate
    metadata:
      name: pihole-tls-cert
    spec:
      dnsNames:
        - "pihole.foo"
      ipAddresses:
        - "10.10.10.10"
      issuerRef:
        group: cert-manager.io
        kind: ClusterIssuer
        name: my-issuer
      secretName: pihole-tls-cert
  - apiVersion: v1
    kind: ConfigMap
    metadata:
      name: lighttpd-external-conf
    data:
      external.conf: |
        $HTTP["host"] =~ "pihole.foo" {
          # Ensure the Pi-hole Block Page knows that this is not a blocked domain
          setenv.add-environment = ("fqdn" => "true")
        
          # Enable the SSL engine with a LE cert, only for this specific host
          $SERVER["socket"] == ":443" {
            ssl.engine = "enable"
            ssl.pemfile = "/etc/ssl/lighttpd-private/tls.crt"
            ssl.privkey = "/etc/ssl/lighttpd-private/tls.key"
            ssl.ca-file = "/etc/ssl/lighttpd-private/ca.crt"
            ssl.honor-cipher-order = "enable"
            ssl.cipher-list = "EECDH+AESGCM:EDH+AESGCM:AES256+EECDH:AES256+EDH"
            ssl.use-sslv2 = "disable"
            ssl.use-sslv3 = "disable"
          }
        }
        # Redirect HTTP to HTTPS
        $HTTP["scheme"] == "http" {
          $HTTP["host"] =~ ".*" {
            url.redirect = (".*" => "https://%0$0")
          }
        }

There should be no backward compatibility issues as this is a totally new field.

fernferret avatar Feb 12 '22 17:02 fernferret

Hi @MoJo2600 I bumped the chart version (I always forget this). I'll bump the other one but that might actually require 2 bumps.

I think you use SemVer so I'd call this a minor (feature) bump (vs a bugfix), or should this have bene a bugfix bump?

fernferret avatar Feb 19 '22 18:02 fernferret

Hmm It looks like #196 was a bugfix bump which added new features. I'm still pretty new to semver (and usually use calver for my projects). But I'd done the minor per the semver spec:

MAJOR version when you make incompatible API changes, MINOR version when you add functionality in a backwards compatible manner, and PATCH version when you make backwards compatible bug fixes.

I'm really cool with whatever y'all want, just need to know which version to bump to pass the CI tests (for this and #211). Thanks again for the awesome helm chart 🚀

fernferret avatar Feb 19 '22 18:02 fernferret

Sorry, I'm always confusing the versioning :( You are right, the last Version should have been a feature bump. I have to pay more attention. It is how you describe it.

MoJo2600 avatar Feb 19 '22 19:02 MoJo2600

No worries at all! I'm just here because I enjoy this chart. Just let me know how you'd like to do versioning (or if you'd like to do it yourself!) and I'll submit changes however you like!

fernferret avatar Feb 20 '22 03:02 fernferret

Hi @MoJo2600 any feedback on this PR?

I'm still using this change actively and it'd be great if this could get merged. If you'd like me to change anything about it, please let me know!

fernferret avatar Nov 19 '22 10:11 fernferret

Hey, sorry for the mucht too late response. I will merge your request now and thank you for your work!

MoJo2600 avatar Nov 23 '22 07:11 MoJo2600