Pass job user as environment variable to task #1465

Merged
merged 4 commits into from Mar 31, 2017

Conversation

Projects
None yet
3 participants
@PtrTeixeira
Contributor

PtrTeixeira commented Mar 20, 2017

Pass the user who clicks the "Run Now" button as the value of the
environment variable STARTED_BY_USER in the task environment. Can provide
more granular, task-level permissions or auditability.

PtrTeixeira added some commits Mar 20, 2017

Pass job user as environment variable to task
Pass the user who clicks the "Run Now" button as the value of the
environment variable `JOB_USER` in the task environment.  Can provide
more granular, task-level permissions or auditability.
@@ -184,6 +184,8 @@ private void prepareEnvironment(final SingularityTaskRequest task, SingularityTa
setEnv(envBldr, "TASK_ID", taskId.getId());
setEnv(envBldr, "ESTIMATED_INSTANCE_COUNT", task.getRequest().getInstancesSafe());
+ setEnv(envBldr, "JOB_USER", task.getPendingTask().getUser().or(""));

This comment has been minimized.

@ssalinas

ssalinas Mar 20, 2017

Member

Two things.

  • Wondering if JOB_USER is descriptive enough here. This method will get called for services and workers as well. Maybe something more like STARTED_BY_USER ?
  • In the case where user is not present on the pending task, would it make more sent to leave this out rather than have an empty string as the value?
@ssalinas

ssalinas Mar 20, 2017

Member

Two things.

  • Wondering if JOB_USER is descriptive enough here. This method will get called for services and workers as well. Maybe something more like STARTED_BY_USER ?
  • In the case where user is not present on the pending task, would it make more sent to leave this out rather than have an empty string as the value?
Fix issues raised in code review
Two small changes:
* Change the name of the environment variable to something
  more descriptive (`JOB_USER` -> `STARTED_BY_USER`)
* Don't set the environment variable at all if the user can't be
  found.
@ssalinas

This comment has been minimized.

Show comment
Hide comment
@ssalinas

ssalinas Mar 21, 2017

Member

👍

Member

ssalinas commented Mar 21, 2017

👍

+ success = success || (environmentVariable.getName().equals("STARTED_BY_USER") && environmentVariable.getValue().equals(user));
+ }
+
+ assertTrue("Expected env variable JOB_USER to be set to " + user, success);

This comment has been minimized.

@darcatron

darcatron Mar 21, 2017

Contributor

got an extra JOB_USER lurking here 🙂

@darcatron

darcatron Mar 21, 2017

Contributor

got an extra JOB_USER lurking here 🙂

@ssalinas ssalinas added the hs_staging label Mar 24, 2017

@PtrTeixeira PtrTeixeira added the hs_qa label Mar 24, 2017

@ssalinas ssalinas added the hs_stable label Mar 30, 2017

@ssalinas ssalinas modified the milestone: 0.15.0 Mar 30, 2017

@ssalinas ssalinas merged commit c2fd507 into master Mar 31, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details

@ssalinas ssalinas deleted the expose-runnow-user branch Mar 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment