Add some helper environment variable to both main and init container (#…

…252)

Add some helper environment variable to both main and init container. This will set the default environment variables USER, UID, HOME, GROUP and GID appropriately from the web host. I.e., HOME is set to the actual users HOME.  This also introduces a breaking change in that we now use maps instead of arrays to define environment variables.

lib/ood_core/job/adapters/kubernetes/batch.rb

@@ -153,26 +153,48 @@ def k8s_username
     username_prefix.nil? ? username : "#{username_prefix}-#{username}"
   end
 
+  def user
+    @user ||= Etc.getpwnam(username)
+  end
+
+  def home_dir
+    user.dir
+  end
+
   def run_as_user
-    Etc.getpwnam(username).uid
+    user.uid
   end
 
   def run_as_group
-    Etc.getpwnam(username).gid
+    user.gid
   end
 
   def fs_group
     run_as_group
   end
 
+  def group
+    Etc.getgrgid(run_as_group).name
+  end
+
+  def default_env
+    {
+      USER: username,
+      UID: run_as_user,
+      HOME: home_dir,
+      GROUP: group,
+      GID: run_as_group,
+    }
+  end
+
   # helper to template resource yml you're going to submit and
   # create an id.
   def generate_id_yml(script)
     native_data = script.native
-    container = helper.container_from_native(native_data[:container])
+    container = helper.container_from_native(native_data[:container], default_env)
     id = generate_id(container.name)
     configmap = helper.configmap_from_native(native_data, id)
-    init_containers = helper.init_ctrs_from_native(native_data[:init_containers])
+    init_containers = helper.init_ctrs_from_native(native_data[:init_containers], container.env)
     spec = OodCore::Job::Adapters::Kubernetes::Resources::PodSpec.new(container, init_containers: init_containers)
     all_mounts = native_data[:mounts].nil? ? mounts : mounts + native_data[:mounts]
 

lib/ood_core/job/adapters/kubernetes/helper.rb

@@ -38,18 +38,21 @@ def info_from_json(pod_json: nil, service_json: nil, secret_json: nil, ns_prefix
   #
   # @param container [#to_h]
   #   the input container hash
+  # @param default_env [#to_h]
+  #   Default env to merge with defined env
   # @return  [OodCore::Job::Adapters::Kubernetes::Resources::Container]
-  def container_from_native(container)
+  def container_from_native(container, default_env)
+    env = container.fetch(:env, {}).to_h.symbolize_keys
     OodCore::Job::Adapters::Kubernetes::Resources::Container.new(
       container[:name],
       container[:image],
       command: parse_command(container[:command]),
       port: container[:port],
-      env: container.fetch(:env, []),
+      env: default_env.merge(env),
       memory: container[:memory],
       cpu: container[:cpu],
       working_dir: container[:working_dir],
-      restart_policy: container[:restart_policy]
+      restart_policy: container[:restart_policy],
     )
   end
 
@@ -93,13 +96,15 @@ def configmap_from_native(native, id)
   # @param native_data [#to_h]
   #   the native data to parse. Expected key init_ctrs and for that
   #   key to be an array of hashes.
+  # @param default_env [#to_h]
+  #   Default env to merge with defined env
   # @return [Array<OodCore::Job::Adapters::Kubernetes::Resources::Container>]
   #   the array of init containers
-  def init_ctrs_from_native(ctrs)
+  def init_ctrs_from_native(ctrs, default_env)
     init_ctrs = []
 
     ctrs&.each do |ctr_raw|
-      ctr = container_from_native(ctr_raw)
+      ctr = container_from_native(ctr_raw, default_env)
       init_ctrs.push(ctr)
     end
 

lib/ood_core/job/adapters/kubernetes/resources.rb

@@ -15,7 +15,7 @@ class Container
                   :restart_policy, :supplemental_groups
 
     def initialize(
-        name, image, command: [], port: nil, env: [], memory: "4Gi", cpu: "1",
+        name, image, command: [], port: nil, env: {}, memory: "4Gi", cpu: "1",
         working_dir: "", restart_policy: "Never", supplemental_groups: []
       )
       raise ArgumentError, "containers need valid names and images" unless name && image
@@ -24,7 +24,7 @@ def initialize(
       @image = image
       @command = command.nil? ? [] : command
       @port = port&.to_i
-      @env = env.nil? ? [] : env
+      @env = env.nil? ? {} : env
       @memory = memory.nil? ? "4Gi" : memory
       @cpu = cpu.nil? ? "1" : cpu
       @working_dir = working_dir.nil? ? "" : working_dir
@@ -44,7 +44,6 @@ def ==(other)
         restart_policy == other.restart_policy &&
         supplemental_groups == other.supplemental_groups
     end
-
   end
 
   class PodSpec

lib/ood_core/job/adapters/kubernetes/templates/pod.yml.erb

@@ -39,13 +39,15 @@ spec:
     <%- unless spec.container.working_dir.empty? -%>
     workingDir: "<%= spec.container.working_dir %>"
     <%- end -%>
-    <%- unless spec.container.env.empty? -%>
     env:
-    <%- spec.container.env.each do |env| -%>
-    - name: <%= env[:name] %>
-      value: "<%= env[:value] %>"
+    - name: POD_NAME
+      valueFrom:
+        fieldRef:
+          fieldPath: metadata.name
+    <%- spec.container.env.each_pair do |name, value| -%>
+    - name: <%= name %>
+      value: "<%= value %>"
     <%- end # for each env -%>
-    <%- end # unless env is nil -%>
     <%- unless spec.container.command.empty? -%>
     command:
     <%- spec.container.command.each do |cmd| -%>
@@ -83,6 +85,15 @@ spec:
   <%- spec.init_containers.each do |ctr| -%>
   - name: "<%= ctr.name %>"
     image: "<%= ctr.image %>"
+    env:
+    - name: POD_NAME
+      valueFrom:
+        fieldRef:
+          fieldPath: metadata.name
+    <%- ctr.env.each_pair do |name, value| -%>
+    - name: <%= name %>
+      value: "<%= value %>"
+    <%- end # for each env -%>
     command:
     <%- ctr.command.each do |cmd| -%>
     - "<%= cmd %>"

spec/fixtures/output/k8s/pod_yml_from_all_configs.yml

@@ -26,8 +26,20 @@ spec:
     imagePullPolicy: IfNotPresent
     workingDir: "/my/home"
     env:
+    - name: POD_NAME
+      valueFrom:
+        fieldRef:
+          fieldPath: metadata.name
+    - name: USER
+      value: "testuser"
+    - name: UID
+      value: "1001"
     - name: HOME
       value: "/my/home"
+    - name: GROUP
+      value: "testgroup"
+    - name: GID
+      value: "1002"
     - name: PATH
       value: "/usr/bin:/usr/local/bin"
     command:
@@ -60,6 +72,23 @@ spec:
   initContainers:
   - name: "init-1"
     image: "busybox:latest"
+    env:
+    - name: POD_NAME
+      valueFrom:
+        fieldRef:
+          fieldPath: metadata.name
+    - name: USER
+      value: "testuser"
+    - name: UID
+      value: "1001"
+    - name: HOME
+      value: "/my/home"
+    - name: GROUP
+      value: "testgroup"
+    - name: GID
+      value: "1002"
+    - name: PATH
+      value: "/usr/bin:/usr/local/bin"
     command:
     - "/bin/ls"
     - "-lrt"

spec/fixtures/output/k8s/pod_yml_from_defaults.yml

@@ -26,8 +26,20 @@ spec:
     imagePullPolicy: IfNotPresent
     workingDir: "/my/home"
     env:
+    - name: POD_NAME
+      valueFrom:
+        fieldRef:
+          fieldPath: metadata.name
+    - name: USER
+      value: "testuser"
+    - name: UID
+      value: "1001"
     - name: HOME
       value: "/my/home"
+    - name: GROUP
+      value: "testgroup"
+    - name: GID
+      value: "1002"
     - name: PATH
       value: "/usr/bin:/usr/local/bin"
     command:
@@ -56,6 +68,23 @@ spec:
   initContainers:
   - name: "init-1"
     image: "busybox:latest"
+    env:
+    - name: POD_NAME
+      valueFrom:
+        fieldRef:
+          fieldPath: metadata.name
+    - name: USER
+      value: "testuser"
+    - name: UID
+      value: "1001"
+    - name: HOME
+      value: "/my/home"
+    - name: GROUP
+      value: "testgroup"
+    - name: GID
+      value: "1002"
+    - name: PATH
+      value: "/usr/bin:/usr/local/bin"
     command:
     - "/bin/ls"
     - "-lrt"

spec/fixtures/output/k8s/pod_yml_no_mounts.yml

@@ -26,8 +26,20 @@ spec:
     imagePullPolicy: IfNotPresent
     workingDir: "/my/home"
     env:
+    - name: POD_NAME
+      valueFrom:
+        fieldRef:
+          fieldPath: metadata.name
+    - name: USER
+      value: "testuser"
+    - name: UID
+      value: "1001"
     - name: HOME
-      value: "/my/home"
+      value: "/home/testuser"
+    - name: GROUP
+      value: "testgroup"
+    - name: GID
+      value: "1002"
     - name: PATH
       value: "/usr/bin:/usr/local/bin"
     command:
@@ -54,6 +66,23 @@ spec:
   initContainers:
   - name: "init-1"
     image: "busybox:latest"
+    env:
+    - name: POD_NAME
+      valueFrom:
+        fieldRef:
+          fieldPath: metadata.name
+    - name: USER
+      value: "testuser"
+    - name: UID
+      value: "1001"
+    - name: HOME
+      value: "/home/testuser"
+    - name: GROUP
+      value: "testgroup"
+    - name: GID
+      value: "1002"
+    - name: PATH
+      value: "/usr/bin:/usr/local/bin"
     command:
     - "/bin/ls"
     - "-lrt"

spec/job/adapters/kubernetes/batch_spec.rb

@@ -6,6 +6,7 @@
   Batch = OodCore::Job::Adapters::Kubernetes::Batch
   Helper = OodCore::Job::Adapters::Kubernetes::Helper
   K8sJobInfo = OodCore::Job::Adapters::Kubernetes::K8sJobInfo
+  User = Struct.new(:dir, :uid, :gid, keyword_init: true)
 
   let(:helper) {
     helper = Helper.new
@@ -275,16 +276,10 @@ def build_script(opts = {})
             image: 'ruby:2.5',
             command: 'rake spec',
             port: 8080,
-            env: [
-              {
-                name: 'HOME',
-                value: '/my/home'
-              },
-              {
-                name: 'PATH',
-                value: '/usr/bin:/usr/local/bin'
-              }
-            ],
+            env: {
+              HOME: '/my/home',
+              PATH: '/usr/bin:/usr/local/bin'
+            },
             memory: '6Gi',
             cpu: '4',
             working_dir: '/my/home',
@@ -311,8 +306,8 @@ def build_script(opts = {})
 
       allow(configured_batch).to receive(:generate_id).with('rspec-test').and_return('rspec-test-123')
       allow(configured_batch).to receive(:username).and_return('testuser')
-      allow(configured_batch).to receive(:run_as_user).and_return(1001)
-      allow(configured_batch).to receive(:run_as_group).and_return(1002)
+      allow(configured_batch).to receive(:user).and_return(User.new(dir: '/home/testuser', uid: 1001, gid: 1002))
+      allow(configured_batch).to receive(:group).and_return('testgroup')
 
       # make sure it get's templated right, also helpful in debugging bc
       # it'll show a better diff than the test below.
@@ -339,16 +334,10 @@ def build_script(opts = {})
             image: 'ruby:2.5',
             command: 'rake spec',
             port: 8080,
-            env: [
-              {
-                name: 'HOME',
-                value: '/my/home'
-              },
-              {
-                name: 'PATH',
-                value: '/usr/bin:/usr/local/bin'
-              }
-            ],
+            env: {
+              'HOME' => '/my/home',
+              'PATH' => '/usr/bin:/usr/local/bin'
+            },
             memory: '6Gi',
             cpu: '4',
             working_dir: '/my/home',
@@ -375,8 +364,8 @@ def build_script(opts = {})
 
       allow(@basic_batch).to receive(:generate_id).with('rspec-test').and_return('rspec-test-123')
       allow(@basic_batch).to receive(:username).and_return('testuser')
-      allow(@basic_batch).to receive(:run_as_user).and_return(1001)
-      allow(@basic_batch).to receive(:run_as_group).and_return(1002)
+      allow(@basic_batch).to receive(:user).and_return(User.new(dir: '/home/testuser', uid: 1001, gid: 1002))
+      allow(@basic_batch).to receive(:group).and_return('testgroup')
 
       # make sure it get's templated right, also helpful in debugging bc
       # it'll show a better diff than the test below.
@@ -403,16 +392,9 @@ def build_script(opts = {})
             image: 'ruby:2.5',
             command: 'rake spec',
             port: 8080,
-            env: [
-              {
-                name: 'HOME',
-                value: '/my/home'
-              },
-              {
-                name: 'PATH',
-                value: '/usr/bin:/usr/local/bin'
-              }
-            ],
+            env: {
+              PATH: '/usr/bin:/usr/local/bin'
+            },
             memory: '6Gi',
             cpu: '4',
             working_dir: '/my/home',
@@ -432,8 +414,8 @@ def build_script(opts = {})
 
       allow(@basic_batch).to receive(:generate_id).with('rspec-test').and_return('rspec-test-123')
       allow(@basic_batch).to receive(:username).and_return('testuser')
-      allow(@basic_batch).to receive(:run_as_user).and_return(1001)
-      allow(@basic_batch).to receive(:run_as_group).and_return(1002)
+      allow(@basic_batch).to receive(:user).and_return(User.new(dir: '/home/testuser', uid: 1001, gid: 1002))
+      allow(@basic_batch).to receive(:group).and_return('testgroup')
 
       # make sure it get's templated right, also helpful in debugging bc
       # it'll show a better diff than the test below.