Skip to content

Commit

Permalink
[Jenson] Refactor Auditing to build upon an object
Browse files Browse the repository at this point in the history
- Previously, it was using context for this purpose
Closes #64
  • Loading branch information
olttwa committed Jan 17, 2019
1 parent c2a571a commit 8aa8297
Show file tree
Hide file tree
Showing 17 changed files with 196 additions and 208 deletions.
32 changes: 13 additions & 19 deletions proctord/audit/auditor.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
package audit

import (
"context"

"github.com/gojektech/proctor/proctord/kubernetes"
"github.com/gojektech/proctor/proctord/logger"
"github.com/gojektech/proctor/proctord/storage"
"github.com/gojektech/proctor/proctord/storage/postgres"
"github.com/gojektech/proctor/proctord/utility"
)

type Auditor interface {
AuditJobsExecution(context.Context)
AuditJobExecutionStatus(string) (string, error)
JobsExecutionAndStatus(*postgres.JobsExecutionAuditLog)
JobsExecution(*postgres.JobsExecutionAuditLog)
JobsExecutionStatus(string) (string, error)
}

type auditor struct {
Expand All @@ -26,29 +26,23 @@ func New(store storage.Store, kubeClient kubernetes.Client) Auditor {
}
}

func (auditor *auditor) AuditJobsExecution(ctx context.Context) {
jobSubmissionStatus := ctx.Value(utility.JobSubmissionStatusContextKey).(string)
userEmail := ctx.Value(utility.UserEmailContextKey).(string)
func (auditor *auditor) JobsExecutionAndStatus(jobsExecutionAuditLog *postgres.JobsExecutionAuditLog) {
auditor.JobsExecution(jobsExecutionAuditLog)

if jobSubmissionStatus != utility.JobSubmissionSuccess {
err := auditor.store.JobsExecutionAuditLog(jobSubmissionStatus, utility.JobFailed, "", userEmail, "", "", map[string]string{})
if err != nil {
logger.Error("Error auditing jobs execution", err)
}
return
if jobsExecutionAuditLog.JobSubmissionStatus == utility.JobSubmissionSuccess &&
jobsExecutionAuditLog.ExecutionID.Valid {
auditor.JobsExecutionStatus(jobsExecutionAuditLog.ExecutionID.String)
}
jobName := ctx.Value(utility.JobNameContextKey).(string)
JobNameSubmittedForExecution := ctx.Value(utility.JobNameSubmittedForExecutionContextKey).(string)
imageName := ctx.Value(utility.ImageNameContextKey).(string)
jobArgs := ctx.Value(utility.JobArgsContextKey).(map[string]string)
}

err := auditor.store.JobsExecutionAuditLog(jobSubmissionStatus, utility.JobWaiting, jobName, userEmail, JobNameSubmittedForExecution, imageName, jobArgs)
func (auditor *auditor) JobsExecution(jobsExecutionAuditLog *postgres.JobsExecutionAuditLog) {
err := auditor.store.AuditJobsExecution(jobsExecutionAuditLog)
if err != nil {
logger.Error("Error auditing jobs execution", err)
}
}

func (auditor *auditor) AuditJobExecutionStatus(jobExecutionID string) (string, error) {
func (auditor *auditor) JobsExecutionStatus(jobExecutionID string) (string, error) {
status, err := auditor.kubeClient.JobExecutionStatus(jobExecutionID)
if err != nil {
logger.Error("Error getting job execution status", err)
Expand Down
13 changes: 8 additions & 5 deletions proctord/audit/auditor_mock.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
package audit

import (
"context"

"github.com/gojektech/proctor/proctord/storage/postgres"
"github.com/stretchr/testify/mock"
)

type MockAuditor struct {
mock.Mock
}

func (m *MockAuditor) AuditJobsExecution(ctx context.Context) {
m.Called(ctx)
func (m *MockAuditor) JobsExecution(JobsExecutionAuditLog *postgres.JobsExecutionAuditLog) {
m.Called(JobsExecutionAuditLog)
}

func (m *MockAuditor) JobsExecutionAndStatus(JobsExecutionAuditLog *postgres.JobsExecutionAuditLog) {
m.Called(JobsExecutionAuditLog)
}

func (m *MockAuditor) AuditJobExecutionStatus(jobExecutionID string) (string, error) {
func (m *MockAuditor) JobsExecutionStatus(jobExecutionID string) (string, error) {
args := m.Called(jobExecutionID)
return args.String(0), args.Error(1)
}
57 changes: 28 additions & 29 deletions proctord/audit/auditor_test.go
Original file line number Diff line number Diff line change
@@ -1,68 +1,67 @@
package audit

import (
"context"
"testing"

"github.com/gojektech/proctor/proctord/kubernetes"
"github.com/gojektech/proctor/proctord/storage"
"github.com/gojektech/proctor/proctord/storage/postgres"
"github.com/gojektech/proctor/proctord/utility"
)

func TestExecutionAuditor(t *testing.T) {
func TestJobsExecutionAuditing(t *testing.T) {
mockStore := &storage.MockStore{}
mockKubeClient := &kubernetes.MockClient{}
testAuditor := New(mockStore, mockKubeClient)
jobsExecutionAuditLog := &postgres.JobsExecutionAuditLog{
JobName: "any-job-name",
}

jobName := "any-job-name"
executedJobName := "proctor-123"
imageName := "any/image:name"
jobArgs := map[string]string{"key": "value"}
userEmail := "mrproctor@example.com"
mockStore.On("AuditJobsExecution", jobsExecutionAuditLog).Return(nil).Once()

ctx := context.WithValue(context.Background(), utility.JobSubmissionStatusContextKey, utility.JobSubmissionSuccess)
ctx = context.WithValue(ctx, utility.JobNameContextKey, jobName)
ctx = context.WithValue(ctx, utility.JobNameSubmittedForExecutionContextKey, executedJobName)
ctx = context.WithValue(ctx, utility.ImageNameContextKey, imageName)
ctx = context.WithValue(ctx, utility.JobArgsContextKey, jobArgs)
ctx = context.WithValue(ctx, utility.UserEmailContextKey, userEmail)

mockStore.On("JobsExecutionAuditLog", utility.JobSubmissionSuccess, utility.JobWaiting, jobName, userEmail, executedJobName, imageName, jobArgs).Return(nil).Once()

testAuditor.AuditJobsExecution(ctx)
testAuditor.JobsExecution(jobsExecutionAuditLog)

mockStore.AssertExpectations(t)
mockKubeClient.AssertExpectations(t)
}

func TestExecutionAuditorClientError(t *testing.T) {
func TestAuditJobsExecutionStatusAuditing(t *testing.T) {
mockStore := &storage.MockStore{}
mockKubeClient := &kubernetes.MockClient{}
testAuditor := New(mockStore, mockKubeClient)
userEmail := "mrproctor@example.com"

ctx := context.WithValue(context.Background(), utility.JobSubmissionStatusContextKey, utility.JobSubmissionClientError)
ctx = context.WithValue(ctx, utility.UserEmailContextKey, userEmail)
jobExecutionID := "job-execution-id"
jobExecutionStatus := "job-execution-status"

mockStore.On("JobsExecutionAuditLog", utility.JobSubmissionClientError, utility.JobFailed, "", userEmail, "", "", map[string]string{}).Return(nil).Once()
mockKubeClient.On("JobExecutionStatus", jobExecutionID).Return(jobExecutionStatus, nil)
mockStore.On("UpdateJobsExecutionAuditLog", jobExecutionID, jobExecutionStatus).Return(nil).Once()

testAuditor.AuditJobsExecution(ctx)
testAuditor.JobsExecutionStatus(jobExecutionID)

mockStore.AssertExpectations(t)
mockKubeClient.AssertExpectations(t)
}

func TestExecutionAuditorServerError(t *testing.T) {
func TestAuditJobsExecutionAndStatusAuditing(t *testing.T) {
mockStore := &storage.MockStore{}
mockKubeClient := &kubernetes.MockClient{}
testAuditor := New(mockStore, mockKubeClient)
userEmail := "mrproctor@example.com"

ctx := context.WithValue(context.Background(), utility.JobSubmissionStatusContextKey, utility.JobSubmissionServerError)
ctx = context.WithValue(ctx, utility.UserEmailContextKey, userEmail)
jobExecutionID := "job-execution-id"
jobExecutionStatus := "job-execution-status"
jobsExecutionAuditLog := &postgres.JobsExecutionAuditLog{
JobName: "any-job-name",
ExecutionID: postgres.StringToSQLString(jobExecutionID),
JobSubmissionStatus: utility.JobSubmissionSuccess,
}

mockStore.On("JobsExecutionAuditLog", utility.JobSubmissionServerError, utility.JobFailed, "", userEmail, "", "", map[string]string{}).Return(nil).Once()
mockStore.On("AuditJobsExecution", jobsExecutionAuditLog).Return(nil).Once()

testAuditor.AuditJobsExecution(ctx)
mockKubeClient.On("JobExecutionStatus", jobExecutionID).Return(jobExecutionStatus, nil)
mockStore.On("UpdateJobsExecutionAuditLog", jobExecutionID, jobExecutionStatus).Return(nil).Once()

testAuditor.JobsExecutionAndStatus(jobsExecutionAuditLog)

mockStore.AssertExpectations(t)
mockKubeClient.AssertExpectations(t)
}
44 changes: 17 additions & 27 deletions proctord/jobs/execution/executioner.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package execution

import (
"context"
"errors"
"fmt"

"github.com/gojektech/proctor/proctord/jobs/metadata"
"github.com/gojektech/proctor/proctord/jobs/secrets"
"github.com/gojektech/proctor/proctord/kubernetes"
"github.com/gojektech/proctor/proctord/logger"
"github.com/gojektech/proctor/proctord/storage/postgres"
"github.com/gojektech/proctor/proctord/utility"
)

Expand All @@ -17,7 +18,7 @@ type executioner struct {
}

type Executioner interface {
Execute(context.Context, string, string, map[string]string) (string, error)
Execute(*postgres.JobsExecutionAuditLog, string, map[string]string) (string, error)
}

func NewExecutioner(kubeClient kubernetes.Client, metadataStore metadata.Store, secretsStore secrets.Store) Executioner {
Expand All @@ -28,41 +29,30 @@ func NewExecutioner(kubeClient kubernetes.Client, metadataStore metadata.Store,
}
}

func (executioner *executioner) Execute(ctx context.Context, jobName, userEmail string, jobArgs map[string]string) (string, error) {
ctx = context.WithValue(ctx, utility.JobNameContextKey, jobName)
ctx = context.WithValue(ctx, utility.UserEmailContextKey, userEmail)
ctx = context.WithValue(ctx, utility.JobArgsContextKey, jobArgs)
func (executioner *executioner) Execute(jobsExecutionAuditLog *postgres.JobsExecutionAuditLog, jobName string, jobArgs map[string]string) (string, error) {
jobsExecutionAuditLog.JobName = jobName

jobMetadata, err := executioner.metadataStore.GetJobMetadata(jobName)
if err != nil {
logger.Error("Error finding job to image", jobName, err.Error())

ctx = context.WithValue(ctx, utility.JobSubmissionStatusContextKey, utility.JobSubmissionServerError)

return "", err
return "", errors.New(fmt.Sprintf("Error finding image for job: %s. Error: %s", jobName, err.Error()))
}

imageName := jobMetadata.ImageName
jobSecrets, err := executioner.secretsStore.GetJobSecrets(jobName)
if err != nil {
//TODO: add check for nil, which means no job secrets configured
logger.Error("Error retrieving secrets for job", jobName, err.Error())
ctx = context.WithValue(ctx, utility.JobSubmissionStatusContextKey, utility.JobSubmissionServerError)
jobsExecutionAuditLog.ImageName = imageName

return "", err
jobSecrets, err := executioner.secretsStore.GetJobSecrets(jobName)
if err != nil && err.Error() != "redigo: nil returned" {
return "", errors.New(fmt.Sprintf("Error retrieving secrets for job: %s. Error: %s", jobName, err.Error()))
}

envVars := utility.MergeMaps(jobArgs, jobSecrets)
jobsExecutionAuditLog.AddJobArgs(envVars)

jobNameSubmittedForExecution, err := executioner.kubeClient.ExecuteJob(imageName, envVars)
jobExecutionID, err := executioner.kubeClient.ExecuteJob(imageName, envVars)
if err != nil {
logger.Error("Error executing job:", jobName, imageName, err.Error())
ctx = context.WithValue(ctx, utility.JobSubmissionStatusContextKey, utility.JobSubmissionServerError)

return "", err
return "", errors.New(fmt.Sprintf("Error submitting job to kube: %s. Error: %s", jobName, err.Error()))
}
ctx = context.WithValue(ctx, utility.JobNameSubmittedForExecutionContextKey, jobNameSubmittedForExecution)
ctx = context.WithValue(ctx, utility.JobSubmissionStatusContextKey, utility.JobSubmissionSuccess)
jobsExecutionAuditLog.AddExecutionID(jobExecutionID)
jobsExecutionAuditLog.JobSubmissionStatus = utility.JobSubmissionSuccess

return jobNameSubmittedForExecution, nil
return jobExecutionID, nil
}
7 changes: 3 additions & 4 deletions proctord/jobs/execution/executioner_mock.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
package execution

import (
"context"

"github.com/gojektech/proctor/proctord/storage/postgres"
"github.com/stretchr/testify/mock"
)

type MockExecutioner struct {
mock.Mock
}

func (m *MockExecutioner) Execute(ctx context.Context, jobName, userEmail string, jobArgs map[string]string) (string, error) {
args := m.Called(ctx, jobName, userEmail, jobArgs)
func (m *MockExecutioner) Execute(jobExecutionAuditLog *postgres.JobsExecutionAuditLog, jobName string, jobArgs map[string]string) (string, error) {
args := m.Called(jobExecutionAuditLog, jobName, jobArgs)
return args.String(0), args.Error(1)
}
31 changes: 16 additions & 15 deletions proctord/jobs/execution/executioner_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package execution

import (
"context"
"errors"
"testing"

"github.com/gojektech/proctor/proctord/jobs/metadata"
"github.com/gojektech/proctor/proctord/jobs/secrets"
"github.com/gojektech/proctor/proctord/kubernetes"
"github.com/gojektech/proctor/proctord/storage/postgres"
"github.com/gojektech/proctor/proctord/utility"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
Expand All @@ -32,8 +32,8 @@ func (suite *ExecutionerTestSuite) SetupTest() {
func (suite *ExecutionerTestSuite) TestSuccessfulJobExecution() {
t := suite.T()

jobsExecutionAuditLog := &postgres.JobsExecutionAuditLog{}
jobName := "sample-job-name"
userEmail := "mrproctor@example.com"
jobArgs := map[string]string{
"argOne": "sample-arg",
}
Expand All @@ -48,27 +48,28 @@ func (suite *ExecutionerTestSuite) TestSuccessfulJobExecution() {
}
suite.mockSecretsStore.On("GetJobSecrets", jobName).Return(jobSecrets, nil).Once()

jobNameSubmittedForExecution := "proctor-ipsum-lorem"
jobExecutionID := "proctor-ipsum-lorem"
envVarsForJob := utility.MergeMaps(jobArgs, jobSecrets)
suite.mockKubeClient.On("ExecuteJob", jobMetadata.ImageName, envVarsForJob).Return(jobNameSubmittedForExecution, nil).Once()
suite.mockKubeClient.On("ExecuteJob", jobMetadata.ImageName, envVarsForJob).Return(jobExecutionID, nil).Once()

executedJobName, err := suite.testExecutioner.Execute(context.Background(), jobName, userEmail, jobArgs)
executedJobName, err := suite.testExecutioner.Execute(jobsExecutionAuditLog, jobName, jobArgs)
assert.NoError(t, err)

suite.mockMetadataStore.AssertExpectations(t)
suite.mockSecretsStore.AssertExpectations(t)
suite.mockKubeClient.AssertExpectations(t)

assert.Equal(t, jobNameSubmittedForExecution, executedJobName)
assert.Equal(t, jobExecutionID, executedJobName)
assert.Equal(t, jobsExecutionAuditLog.JobName, jobName)
}

func (suite *ExecutionerTestSuite) TestJobExecutionOnImageLookupFailure() {
t := suite.T()

suite.mockMetadataStore.On("GetJobMetadata", mock.Anything).Return(&metadata.Metadata{}, errors.New("No image found for job name")).Once()
suite.mockMetadataStore.On("GetJobMetadata", mock.Anything).Return(&metadata.Metadata{}, errors.New("image-fetch-error")).Once()

_, err := suite.testExecutioner.Execute(context.Background(), "any-job", "foo@bar.com", map[string]string{})
assert.EqualError(t, err, "No image found for job name")
_, err := suite.testExecutioner.Execute(&postgres.JobsExecutionAuditLog{}, "any-job", map[string]string{})
assert.EqualError(t, err, "Error finding image for job: any-job. Error: image-fetch-error")
}

func (suite *ExecutionerTestSuite) TestJobExecutionOnSecretsFetchFailure() {
Expand All @@ -77,10 +78,10 @@ func (suite *ExecutionerTestSuite) TestJobExecutionOnSecretsFetchFailure() {
jobMetadata := metadata.Metadata{ImageName: "img"}
suite.mockMetadataStore.On("GetJobMetadata", mock.Anything).Return(&jobMetadata, nil).Once()

suite.mockSecretsStore.On("GetJobSecrets", mock.Anything).Return(map[string]string{}, errors.New("secrets fetch error")).Once()
suite.mockSecretsStore.On("GetJobSecrets", mock.Anything).Return(map[string]string{}, errors.New("secret-store-error")).Once()

_, err := suite.testExecutioner.Execute(context.Background(), "any-job", "foo@bar.com", map[string]string{})
assert.EqualError(t, err, "secrets fetch error")
_, err := suite.testExecutioner.Execute(&postgres.JobsExecutionAuditLog{}, "any-job", map[string]string{})
assert.EqualError(t, err, "Error retrieving secrets for job: any-job. Error: secret-store-error")
}

func (suite *ExecutionerTestSuite) TestJobExecutionOnKubernetesJobExecutionFailure() {
Expand All @@ -90,11 +91,11 @@ func (suite *ExecutionerTestSuite) TestJobExecutionOnKubernetesJobExecutionFailu
suite.mockMetadataStore.On("GetJobMetadata", mock.Anything).Return(&jobMetadata, nil).Once()

suite.mockSecretsStore.On("GetJobSecrets", mock.Anything).Return(map[string]string{}, nil).Once()
suite.mockKubeClient.On("ExecuteJob", mock.Anything, mock.Anything).Return("", errors.New("Kube client job execution error")).Once()
suite.mockKubeClient.On("ExecuteJob", mock.Anything, mock.Anything).Return("", errors.New("kube-client-error")).Once()

_, err := suite.testExecutioner.Execute(context.Background(), "any-job", "foo@bar.com", map[string]string{})
_, err := suite.testExecutioner.Execute(&postgres.JobsExecutionAuditLog{}, "any-job", map[string]string{})

assert.EqualError(t, err, "Kube client job execution error")
assert.EqualError(t, err, "Error submitting job to kube: any-job. Error: kube-client-error")
}

func TestExecutionerTestSuite(t *testing.T) {
Expand Down
Loading

0 comments on commit 8aa8297

Please sign in to comment.