garethr / garethr-kubernetes

Puppet types and provider for managing Pods, ReplicationControllers, Services and more in Kubernetes
http://garethr.github.io/garethr-kubernetes
Apache License 2.0
28 stars 28 forks source link

Adding the same secret (with the same name) in multiple namespaces #45

Open pedrorochagoncalves opened 5 years ago

pedrorochagoncalves commented 5 years ago

Hey,

First of all I wanted to thank you very much for having created this module, it's very useful and I've been using it at the company I work for.

We seem to have struck a small issue and I wanted to request your help and opinion. It would be great if we could add the same secret, with the same content and name, to multiple namespaces. So initially we had the following manifest:

class some_kubernetes_class(
  Hash[String, Hash] $secrets,
  Array[String] $namespaces
) {
  # In hiera:
  # some_kubernetes_class::secrets:
  #   <secret_name>:
  #     ensure: <ensure>
  #     all_namespaces: <false|true>
  #     metadata:
  #       name: supername
  #       namespace: supernamespace
  #       otherkey: othervalue
  #       otherhashkey:
  #         key: value
  #     data:
  #       key1: value1
  #       key2: value2
  #     type: <type> 

  $secrets.each |$secretname, $secret| {
    # if the same secret needs to exist in all namespaces
    if str2bool($secret['all_namespaces']) {
      $namespaces.each |$namespace| {
        $metadata = $secret['metadata'] + { 'namespace' => $namespace }
        kubernetes_secret { "${secretname}-${namespace}":
          ensure   => $secret['ensure'],
          metadata => $metadata,
          data     => $secret['data'],
          type     => $secret['type']
        }
      }
    }
    else {
      kubernetes_secret { $secretname:
        ensure   => $secret['ensure'],
        metadata => $secret['metadata'],
        data     => $secret['data'],
        type     => $secret['type']
      }
    }
  }
}

Suppose we have the following hash in hiera for the secrets we want to add:

some_kubernetes_class::namespaces:
  - namespace1
  - namespace2
  - namespace3
some_kubernetes_class::secrets:
  docker_reg:
    ensure: present
    all_namespaces: true
    metadata:
      name: docker_reg
    data:
      ".config.json": <some_base64_stuff>
    type: "kubernetes.io/dockerconfigjson"
  web-tls-dhparam:
    ensure: present
    all_namespaces: false
    metadata:
      name: "web-tls-dhparam"
      namespace: "namespace1"
    data:
      "dhparam.pem": <some_base64_stuff>
    type: "Opaque"

The second secret, web-tls-dhparam, will be created and managed just fine. We only manage it in one namespace, so the resulting Puppet resource would look like this:

kubernetes_secret { "web-tls-dhparam":
        ensure   => "present",
        metadata => {"name" => "web-tls-dhparam", "namespace" => "namespace1"},
        data     => {"dhparam.pem" => "<some_baes64_stuff>"},
        type     => "Opaque"
      }

The first secret, docker_reg, which needs to be created with the same name on all namespaces is where this gets tricky. These are the resulting Puppet resources:

kubernetes_secret { "docker-reg-namespace1":
        ensure   => "present",
        metadata => {"name" => "docker-reg", "namespace" => "namespace1"},
        data     => {".config.json" => "<some_baes64_stuff>"},
        type     => "kubernetes.io/dockerconfigjson"
      }
kubernetes_secret { "docker-reg-namespace2":
        ensure   => "present",
        metadata => {"name" => "docker-reg", "namespace" => "namespace2"},
        data     => {".config.json" => "<some_baes64_stuff>"},
        type     => "kubernetes.io/dockerconfigjson"
      }
kubernetes_secret { "docker-reg-namespace3":
        ensure   => "present",
        metadata => {"name" => "docker-reg", "namespace" => "namespace3"},
        data     => {".config.json" => "<some_baes64_stuff>"},
        type     => "kubernetes.io/dockerconfigjson"
      }

The issue is that the kubernetes_secret provider/type uses the content of the name variable (which is the same as the title of the resource in this case) to name the secret instead of what is inside metadata.name. So, Puppet will attempt to create 3 secrets called docker-reg-namespace1, docker-reg-namespace2 and docker-reg-namespac3 on namespace1, namespace2 and namespace3 respectively. But what we want, is a secret called docker-reg on all three namespaces. And we can't change the content of the name param, because it needs to match the title.

I'm a complete noob in Puppet custom resources, providers, types, etc. But I tried to hack around a little bit because I really wanted this to work. I started reading and found about composite namevars, which sounded exactly like what I needed to fix this. So I dove into the code and added the following:

(I'm aware these files are auto generated by puppet-swagger-generator, but I still wanted to hack around).

lib/puppet/type/kubernetes_secret.rb
Puppet::Type.newtype(:kubernetes_secret) do

  @doc = "Secret holds secret data of a certain type. The total bytes of the values in the Data field must be less than MaxSecretSize bytes."

  ensurable

  def self.title_patterns
    [
      [ /(^([^\/]*)$)/m,
        [ [:name] ] ],
      [ /^([^\/]+)\/([^\/]+)$/,
        [ [:namespace], [:name] ]
      ]
    ]
  end

  newparam(:name, namevar: true) do
    desc 'Name of the secret.'
  end

  newparam(:namespace, namevar: true) do
    desc 'Namespace of the secret.'
  end
lib/puppet/provider/kubernetes_secret/swagger.rb
Puppet::Type.type(:kubernetes_secret).provide(:swagger, :parent => PuppetX::Puppetlabs::Kubernetes::Provider) do

  mk_resource_methods

  def self.instance_to_hash(instance)
    {
    ensure: :present,
    name: instance.metadata.name,
    namespace: instance.metadata.namespace,

        metadata: instance.metadata.respond_to?(:to_hash) ? instance.metadata.to_hash : instance.metadata,

        data: instance.data.respond_to?(:to_hash) ? instance.data.to_hash : instance.data,

        type: instance.type.respond_to?(:to_hash) ? instance.type.to_hash : instance.type,

    object: instance,
    }

(...)

  def build_params
    params = {

        namespace: resource[:namespace],

        metadata: resource[:metadata],

        data: resource[:data],

        type: resource[:type],

    }
    params.delete_if { |key, value| value.nil? }
    params
  end

Having a composite namevar would allow me to change the Puppet code to the following:

 $secrets.each |$secretname, $secret| {
    # if the same secret needs to exist in all namespaces
    if str2bool($secret['all_namespaces']) {
      $namespaces.each |$namespace| {
        $metadata = $secret['metadata'] + { 'namespace' => $namespace }
        kubernetes_secret { "${secretname}-${namespace}":
          ensure        => $secret['ensure'],
          name           => $secretname,
          namespace => $namespace,
          metadata     => $metadata,
          data              => $secret['data'],
          type              => $secret['type']
        }
      }
    }
    else {
      kubernetes_secret { $secretname:
        ensure         => $secret['ensure'],
        name           => $secretname,
        namespace => $namespace,
        metadata     => $secret['metadata'],
        data             => $secret['data'],
        type             => $secret['type']
      }
    }
  }

Aha! So now for the docker-reg secret case that needs to exist in all namespaces, we can have the name parameter be the same for all three, because the resource now has a composite namevar:

kubernetes_secret { "docker-reg-namespace1":
        ensure   => "present",
        name        => "docker-reg",
        namespace => "namespace1",
        metadata => {"name" => "docker-reg", "namespace" => "namespace1"},
        data     => {"dhparam.pem" => "<some_baes64_stuff>"},
        type     => "kubernetes.io/dockerconfigjson"
      }
kubernetes_secret { "docker-reg-namespace2":
        ensure   => "present",
        name           => "docker-reg",
        namespace => "namespace2",
        metadata => {"name" => "docker-reg", "namespace" => "namespace2"},
        data     => {"dhparam.pem" => "<some_baes64_stuff>"},
        type     => "kubernetes.io/dockerconfigjson"
      }

This works....partially. And this is the part where things are very fuzzy for me. 😀

Info: Checking if docker-reg exists
Info: Exists: {}
Info: Creating kubernetes_secret docker-reg
Error: Could not set 'present' on ensure: HTTP status code 404, namespaces "absent" not found (file: /etc/puppetlabs/code/environments/pedro_env/modules/te_kubernetes/manifests/configure/master/secrets.pp, line: 27)
Error: Could not set 'present' on ensure: HTTP status code 404, namespaces "absent" not found (file: /etc/puppetlabs/code/environments/pedro_env/modules/te_kubernetes/manifests/configure/master/secrets.pp, line: 27)
Wrapped exception:
namespaces "absent" not found
Error: /Stage[main]/Te_kubernetes::Configure::Master::Secrets/Kubernetes_secret[te-docker-reg-webapps]/ensure: change from 'absent' to 'present' failed: Could not set 'present' on ensure: HTTP status code 404, namespaces "absent" not found (file: /etc/puppetlabs/code/environments/pedro_env/modules/te_kubernetes/manifests/configure/master/secrets.pp, line: 27)

In this scenario the secrets already exist in all namespaces. Puppet can't seem to find them (the exists info was added by me to check the content of @property_hash). I can see them being returned in the self.instances function of the provider, but I have no clue why the @property_hash is empty for them. I'm also clueless about the error above. Where is it getting the absent namespace from? I have no idea how to debug that.

Interestingly though, this does work just fine for the other secret that doesn't need to exist in all namespaces, which confuses me further. 😄

I was wondering if you could help me figure out what's going on and would like to hear your opinion. I realize the files I changed are auto generated, so maybe you have a much better idea on how to tackle this use case, to allow us to deploy the same secret with the same secret name to multiple namespaces. I apologize for the wall of text but I wanted to give you as much information as possible.

Thank you and I look forward to hearing back from you.

Cheers,

MikaelSmith commented 5 years ago

The composite namevar does seem to make sense here.

For not finding them, could list_instances_of be omitting the namespace? Then they wouldn't match because the composite namevar is used for matching.

Could absent be introduced somehow because your else clause includes namespace => $namespace and $namespace is undef? What particular code does line 27 correspond to?

I suspect the auto-generation needs an update to account for namespaces across all resources.

pedrorochagoncalves commented 5 years ago

@MikaelSmith , thank you very much for taking the time to read my wall of text and helping!

So, I checked what kubeclient returns and it seems to be returning the namespace as expected. Example:

#<Kubeclient::Resource metadata={:name=>"te-docker-reg", :namespace=>"ops", :selfLink=>"/api/v1/namespaces/ops/secrets/te-docker-reg", :uid=>"<some_id>", :resourceVersion=>"311", :creationTimestamp=>"2018-12-13T16:57:40Z"}, data={:".dockerconfigjson"=>"some_base64_stuff"}....

And actually, I printed the content of the variable hash inside the self.instances (lib/puppet_x/swagger/provider.rb):

        def self.instances
          begin
            list_instances.collect do |instance|
              begin
                hash = instance_to_hash(instance)
                Puppet.debug("Ignoring #{name} due to invalid or incomplete response") unless hash
                new(hash) if hash
              end
            end.compact
          rescue Timeout::Error, StandardError => e
            raise PuppetX::Puppetlabs::Swagger::PrefetchError.new(self.resource_type.name.to_s, e)
          end
        end

and it seems to be populating the fields in the hash according to what is defined in lib/puppet/provider/kubernetes_secret/swagger.rb inside self.instance_to_hash (as posted in the original post) as expected:

Info: Hash: {:ensure=>:present, :name=>"te-docker-reg", :namespace=>"ops", :metadata=>{:name=>"te-docker-reg", :namespace=>"ops", :selfLink=>"/api/v1/namespaces/ops/secrets/te-docker-reg", :uid=>"<some_uid>", :resourceVersion=>"311", :creationTimestamp=>"2018-12-13T16:57:40Z"}, :data=>{:".dockerconfigjson"=>"<base64_stuff>"....}

But then the @property_hash variable is empty it seems.

As regards the absent, thank you for pointing out a bug. You're right that the $namespace variable inside the else clause is undef and I've fixed that! 👍 Unfortunately, however, the errors I posted above seem to happen only for the secrets that go in the if clause and not the else clause. The secrets that go in the else clause seem to be working just fine and they are matched. Which makes that error even more of a mystery to me. Line 27 corresponds to: kubernetes_secret { "${secretname}-${namespace}":, which is the first line of:

        kubernetes_secret { "${secretname}-${namespace}":
          ensure   => $secret['ensure'],
          name => $secretname,
          namespace => $namespace,
          metadata => $metadata,
          data     => $secret['data'],
          type     => $secret['type']
        }

Once again, thank you for your time and patience!

MikaelSmith commented 5 years ago

Could it be referring to a namespace that doesn't exist? I don't really have a lot of ideas.

pedro-te commented 5 years ago

Based on what I've seen, I don't think that it's referring to a non-existing namespace. I was printing the namespaces during execution and they were all existing namespaces.

I don't have more ideas either unfortunately... Anyway, I really appreciate your help!