Merge pull request #79 from jgnagy/refactor/changes-to-noisy-resources

Refactoring Service and Secret templates to be less noisy

Author: jgnagy

Gemfile.lock

@@ -2,6 +2,7 @@ PATH
   remote: .
   specs:
     metatron (0.10.1)
+      base64
       json (~> 2.6)
       rack (>= 2.2.8, < 4)
 
@@ -10,6 +11,7 @@ GEM
   specs:
     ast (2.4.2)
     backport (1.2.0)
+    base64 (0.2.0)
     benchmark (0.4.0)
     byebug (11.1.3)
     diff-lcs (1.6.0)

lib/metatron.rb

@@ -6,6 +6,9 @@
 require "time"
 require "logger"
 
+# external requirements
+require "base64"
+
 # The top-level module for Metatron
 module Metatron
   class Error < StandardError; end

lib/metatron/templates/secret.rb

@@ -7,15 +7,38 @@ class Secret < Template
       include Concerns::Annotated
       include Concerns::Namespaced
 
-      attr_accessor :additional_labels, :type, :data
+      attr_accessor :additional_labels, :type, :data, :string_data
 
-      def initialize(name, data = {})
+      alias stringData string_data
+
+      def initialize(name, provided_string_data = nil, string_data: nil, data: nil)
         super(name)
+        @string_data = string_data || provided_string_data
         @data = data
         @additional_labels = {}
         @type = "Opaque"
       end
 
+      def formatted_data
+        data_hash = {}
+
+        # Only include one of data or string_data, preferring data if both are present
+        return data_hash unless data || string_data
+
+        # If string_data is provided, it should be a hash of string values
+        # and will be base64 encoded
+        if string_data
+          data_hash[:data] = string_data.transform_values do |value|
+            Base64.strict_encode64(value.to_s).gsub("\n", "")
+          end
+        end
+
+        # If data is provided, it should be a hash of base64 encoded values
+        data_hash[:data] = data if data
+
+        data_hash
+      end
+
       def render
         {
           apiVersion:,
@@ -24,9 +47,8 @@ def render
             name:,
             labels: base_labels.merge(additional_labels)
           }.merge(formatted_annotations).merge(formatted_namespace).compact,
-          type:,
-          stringData: data
-        }.compact
+          type:
+        }.compact.merge(formatted_data).compact
       end
     end
   end

lib/metatron/templates/service.rb

@@ -40,6 +40,10 @@ def formatted_ports
         ports&.any? ? { ports: } : {}
       end
 
+      def formatted_publish_not_ready_addresses
+        publish_not_ready_addresses ? { publishNotReadyAddresses: } : {}
+      end
+
       def render
         {
           apiVersion:,
@@ -51,9 +55,8 @@ def render
           spec: {
             type:,
             selector: selector.merge(additional_selector_labels),
-            publishNotReadyAddresses:,
             clusterIP:
-          }.compact.merge(formatted_ports)
+          }.compact.merge(formatted_ports).merge(formatted_publish_not_ready_addresses)
         }
       end
     end

metatron.gemspec

@@ -27,6 +27,7 @@ Gem::Specification.new do |spec|
 
   spec.required_ruby_version = "~> 3.1"
 
+  spec.add_dependency "base64"
   spec.add_dependency "json", "~> 2.6"
   spec.add_dependency "rack", ">= 2.2.8", "< 4"
 

spec/metatron/templates/secret_spec.rb

@@ -1,28 +1,142 @@
 # frozen_string_literal: true
 
 RSpec.describe Metatron::Templates::Secret do
-  let(:secret) { described_class.new("test", { "some secret" => "value" }) }
-
-  let(:rendered_secret) do
-    {
-      apiVersion: "v1",
-      kind: "Secret",
-      metadata: {
-        name: "test",
-        labels: { "metatron.therubyist.org/name": "test" }
-      },
-      type: "Opaque",
-      stringData: {
-        "some secret" => "value"
+  describe "using legacy unnamed constructor with string_data" do
+    let(:secret) { described_class.new("test", { "some secret" => "value" }) }
+
+    let(:rendered_secret) do
+      {
+        apiVersion: "v1",
+        kind: "Secret",
+        metadata: {
+          name: "test",
+          labels: { "metatron.therubyist.org/name": "test" }
+        },
+        type: "Opaque",
+        data: { "some secret" => "dmFsdWU=" } # Base64 encoded "value"
       }
-    }
+    end
+
+    it "produces a hash" do
+      expect(secret.render).to be_a(Hash)
+    end
+
+    it "renders properly" do
+      expect(secret.render).to eq(rendered_secret)
+    end
   end
 
-  it "produces a hash" do
-    expect(secret.render).to be_a(Hash)
+  describe "using named constructor with string_data" do
+    let(:secret) do
+      described_class.new("test", string_data: { "some secret" => "value" })
+    end
+
+    let(:rendered_secret) do
+      {
+        apiVersion: "v1",
+        kind: "Secret",
+        metadata: {
+          name: "test",
+          labels: { "metatron.therubyist.org/name": "test" }
+        },
+        type: "Opaque",
+        data: { "some secret" => "dmFsdWU=" } # Base64 encoded "value"
+      }
+    end
+
+    it "produces a hash" do
+      expect(secret.render).to be_a(Hash)
+    end
+
+    it "renders properly" do
+      expect(secret.render).to eq(rendered_secret)
+    end
   end
 
-  it "renders properly" do
-    expect(secret.render).to eq(rendered_secret)
+  describe "using named constructor with data" do
+    let(:secret) do
+      described_class.new("test", data: { "some secret" => "b3RoZXIgdmFsdWU=" })
+    end
+
+    let(:rendered_secret) do
+      {
+        apiVersion: "v1",
+        kind: "Secret",
+        metadata: {
+          name: "test",
+          labels: { "metatron.therubyist.org/name": "test" }
+        },
+        type: "Opaque",
+        data: { "some secret" => "b3RoZXIgdmFsdWU=" } # Base64 encoded "other value"
+      }
+    end
+
+    it "produces a hash" do
+      expect(secret.render).to be_a(Hash)
+    end
+
+    it "renders properly" do
+      expect(secret.render).to eq(rendered_secret)
+    end
+  end
+
+  describe "using named constructor with both string_data and data" do
+    let(:secret) do
+      described_class.new(
+        "test",
+        data: { "some secret" => "b3RoZXIgdmFsdWU=" }, # should prefer this
+        string_data: { "some secret" => "value" }
+      )
+    end
+
+    let(:rendered_secret) do
+      {
+        apiVersion: "v1",
+        kind: "Secret",
+        metadata: {
+          name: "test",
+          labels: { "metatron.therubyist.org/name": "test" }
+        },
+        type: "Opaque",
+        data: { "some secret" => "b3RoZXIgdmFsdWU=" } # Base64 encoded "other value"
+      }
+    end
+
+    it "produces a hash" do
+      expect(secret.render).to be_a(Hash)
+    end
+
+    it "renders properly" do
+      expect(secret.render).to eq(rendered_secret)
+    end
+  end
+
+  describe "using no data or string_data in constructor, applying string_data later" do
+    let(:secret) do
+      r = described_class.new("test")
+      r.string_data = { "some secret" => "value" }
+      r
+    end
+
+    let(:rendered_secret) do
+      {
+        apiVersion: "v1",
+        kind: "Secret",
+        metadata: {
+          name: "test",
+          labels: { "metatron.therubyist.org/name": "test" }
+        },
+        type: "Opaque",
+        data: { "some secret" => "dmFsdWU=" } # Base64 encoded "value"
+      }
+    end
+
+    it "produces a hash" do
+      expect(secret.render).to be_a(Hash)
+    end
+
+    it "renders properly" do
+      expect(secret.render).to eq(rendered_secret)
+    end
   end
 end

spec/metatron/templates/service_spec.rb

@@ -16,7 +16,6 @@
         name: "test"
       },
       spec: {
-        publishNotReadyAddresses: false,
         selector: { "foo.bar/name": "test" },
         type: "ClusterIP",
         clusterIP: "None",
@@ -36,7 +35,25 @@
         name: "test"
       },
       spec: {
-        publishNotReadyAddresses: false,
+        selector: { "foo.bar/name": "test" },
+        type: "ClusterIP",
+        ports: [
+          { name: "test", port: 3306, protocol: "TCP", targetPort: 3306 }
+        ]
+      }
+    }
+  end
+
+  let(:rendered_service_with_publish_not_ready_addresses) do
+    {
+      apiVersion: "v1",
+      kind: "Service",
+      metadata: {
+        labels: { "metatron.therubyist.org/name": "test" },
+        name: "test"
+      },
+      spec: {
+        publishNotReadyAddresses: true,
         selector: { "foo.bar/name": "test" },
         type: "ClusterIP",
         ports: [
@@ -58,4 +75,9 @@
     service.cluster_ip = "None"
     expect(service.render).to eq(rendered_service_with_cluster_ip_none)
   end
+
+  it "renders properly with publishNotReadyAddresses: true" do
+    service.publish_not_ready_addresses = true
+    expect(service.render).to eq(rendered_service_with_publish_not_ready_addresses)
+  end
 end