Fix --expires-on flag parsing

CHANGELOG.md

@@ -8,6 +8,7 @@
 - Accept YAML configuration and env vars for `dce system deploy` params for namespace and budget notification emails.
 - Fix bug where modified configuration values used by `dce system deploy` would not be reflected in the terraform deployment
 - Fix pre-run credentials check: was accepting expired credentials.
+- Fix parsing of `--expires-on` flag for `dce leases create` command
 
 ## v0.4.0
 - Added `--tf-init-options` and `--tf-apply-options` for greater control over underlying provisioner

pkg/service/accounts.go

@@ -24,7 +24,7 @@ func (s *AccountsService) AddAccount(accountID, adminRoleARN string) {
 		},
 	}
 	params.SetTimeout(5 * time.Second)
-	_, err := apiClient.PostAccounts(params, nil)
+	_, err := ApiClient.PostAccounts(params, nil)
 	if err != nil {
 		log.Fatalln("err: ", err)
 	} else {
@@ -37,7 +37,7 @@ func (s *AccountsService) RemoveAccount(accountID string) {
 		ID: accountID,
 	}
 	params.SetTimeout(5 * time.Second)
-	_, err := apiClient.DeleteAccountsID(params, nil)
+	_, err := ApiClient.DeleteAccountsID(params, nil)
 	if err != nil {
 		log.Fatalln("err: ", err)
 	} else {
@@ -50,15 +50,15 @@ func (s *AccountsService) GetAccount(accountID string) {
 		ID: accountID,
 	}
 	params.SetTimeout(5 * time.Second)
-	res, err := apiClient.GetAccountsID(params, nil)
+	res, err := ApiClient.GetAccountsID(params, nil)
 	if err != nil {
 		log.Fatalln("err: ", err)
 	}
 	jsonPayload, err := json.MarshalIndent(res.GetPayload(), "", "\t")
 	if err != nil {
 		log.Fatalln("err: ", err)
 	}
-	if _, err := out.Write(jsonPayload); err != nil {
+	if _, err := Out.Write(jsonPayload); err != nil {
 		log.Fatalln("err: ", err)
 	}
 }
@@ -67,15 +67,15 @@ func (s *AccountsService) GetAccount(accountID string) {
 func (s *AccountsService) ListAccounts() {
 	params := &operations.GetAccountsParams{}
 	params.SetTimeout(5 * time.Second)
-	res, err := apiClient.GetAccounts(params, nil)
+	res, err := ApiClient.GetAccounts(params, nil)
 	if err != nil {
 		log.Fatalln("err: ", err)
 	}
 	jsonPayload, err := json.MarshalIndent(res.GetPayload(), "", "\t")
 	if err != nil {
 		log.Fatalln("err: ", err)
 	}
-	if _, err := out.Write(jsonPayload); err != nil {
+	if _, err := Out.Write(jsonPayload); err != nil {
 		log.Fatalln("err: ", err)
 	}
 }

pkg/service/leases.go

@@ -31,7 +31,7 @@ func (s *LeasesService) CreateLease(principalID string, budgetAmount float64, bu
 
 	expiry, err := s.Util.ExpandEpochTime(expiresOn)
 
-	if err != nil && expiry > 0 {
+	if err == nil && expiry > 0 {
 		expiryf := float64(expiry)
 		postBody.ExpiresOn = expiryf
 	}
@@ -40,7 +40,7 @@ func (s *LeasesService) CreateLease(principalID string, budgetAmount float64, bu
 		Lease: postBody,
 	}
 	params.SetTimeout(5 * time.Second)
-	res, err := apiClient.PostLeases(params, nil)
+	res, err := ApiClient.PostLeases(params, nil)
 	if err != nil {
 		log.Fatalln("err: ", err)
 	}
@@ -50,7 +50,7 @@ func (s *LeasesService) CreateLease(principalID string, budgetAmount float64, bu
 	}
 	log.Infoln("Lease created.")
 
-	if _, err := out.Write(jsonPayload); err != nil {
+	if _, err := Out.Write(jsonPayload); err != nil {
 		log.Fatalln("err: ", err)
 
 	}
@@ -64,7 +64,7 @@ func (s *LeasesService) EndLease(accountID, principalID string) {
 		},
 	}
 	params.SetTimeout(5 * time.Second)
-	_, err := apiClient.DeleteLeases(params, nil)
+	_, err := ApiClient.DeleteLeases(params, nil)
 	if err != nil {
 		log.Fatalln("err: ", err)
 	}
@@ -76,15 +76,15 @@ func (s *LeasesService) GetLease(leaseID string) {
 		ID: leaseID,
 	}
 	params.SetTimeout(5 * time.Second)
-	res, err := apiClient.GetLeasesID(params, nil)
+	res, err := ApiClient.GetLeasesID(params, nil)
 	if err != nil {
 		log.Fatalln("err: ", err)
 	}
 	jsonPayload, err := json.MarshalIndent(res.GetPayload(), "", "\t")
 	if err != nil {
 		log.Fatalln("err: ", err)
 	}
-	if _, err := out.Write(jsonPayload); err != nil {
+	if _, err := Out.Write(jsonPayload); err != nil {
 		log.Fatalln("err: ", err)
 	}
 }
@@ -99,15 +99,15 @@ func (s *LeasesService) ListLeases(acctID, principalID, nextAcctID, nextPrincipa
 		Status:          &leaseStatus,
 	}
 	params.SetTimeout(5 * time.Second)
-	res, err := apiClient.GetLeases(params, nil)
+	res, err := ApiClient.GetLeases(params, nil)
 	if err != nil {
 		log.Fatalln("err: ", err)
 	}
 	jsonPayload, err := json.MarshalIndent(res.GetPayload(), "", "\t")
 	if err != nil {
 		log.Fatalln("err: ", err)
 	}
-	if _, err := out.Write(jsonPayload); err != nil {
+	if _, err := Out.Write(jsonPayload); err != nil {
 		log.Fatalln("err: ", err)
 	}
 }
@@ -125,7 +125,7 @@ func (s *LeasesService) Login(opts *LeaseLoginOptions) {
 
 	params := &operations.PostLeasesAuthParams{}
 	params.SetTimeout(20 * time.Second)
-	res, err := apiClient.PostLeasesAuth(params, nil)
+	res, err := ApiClient.PostLeasesAuth(params, nil)
 
 	if err != nil {
 		log.Fatal(err)
@@ -143,7 +143,7 @@ func (s *LeasesService) LoginByID(leaseID string, opts *LeaseLoginOptions) {
 		ID: leaseID,
 	}
 	params.SetTimeout(20 * time.Second)
-	res, err := apiClient.PostLeasesIDAuth(params, nil)
+	res, err := ApiClient.PostLeasesIDAuth(params, nil)
 	if err != nil {
 		log.Fatalln("err: ", err)
 	}
@@ -177,7 +177,7 @@ func (s *LeasesService) loginWithCreds(leaseCreds *leaseCreds, opts *LeaseLoginO
 			leaseCreds.AccessKeyID,
 			leaseCreds.SecretAccessKey,
 			leaseCreds.SessionToken)
-		if _, err := out.Write([]byte(creds)); err != nil {
+		if _, err := Out.Write([]byte(creds)); err != nil {
 			log.Fatalln("err: ", err)
 		}
 	}

pkg/service/service.go

@@ -20,15 +20,15 @@ type ServiceContainer struct {
 }
 
 var log observ.Logger
-var apiClient utl.APIer
-var out observ.OutputWriter
+var ApiClient utl.APIer
+var Out observ.OutputWriter
 
 // New returns a new ServiceContainer given config
 func New(config *configs.Root, observation *observ.ObservationContainer, util *utl.UtilContainer) *ServiceContainer {
 
 	log = observation.Logger
-	apiClient = util.APIer
-	out = observation.OutputWriter
+	ApiClient = util.APIer
+	Out = observation.OutputWriter
 
 	serviceContainer := ServiceContainer{
 		Config:        config,

pkg/service/usage.go

@@ -22,15 +22,15 @@ func (s *UsageService) GetUsage(startDate, endDate float64) {
 		EndDate:   endDate,
 	}
 	params.SetTimeout(5 * time.Second)
-	res, err := apiClient.GetUsage(params, nil)
+	res, err := ApiClient.GetUsage(params, nil)
 	if err != nil {
 		log.Fatalln("err: ", err)
 	} else {
 		jsonPayload, err := json.MarshalIndent(res.GetPayload(), "", "\t")
 		if err != nil {
 			log.Fatalln("err: ", err)
 		}
-		if _, err := out.Write(jsonPayload); err != nil {
+		if _, err := Out.Write(jsonPayload); err != nil {
 			log.Fatalln("err: ", err)
 		}
 	}

tests/integration/cli_test_util.go

@@ -23,15 +23,31 @@ type cliTest struct {
 	stdout     *bytes.Buffer
 	injector   injector
 	configFile string
+	t          *testing.T
 }
 
 func (test *cliTest) WriteConfig(t *testing.T, config *configs.Root) {
 	test.configFile = writeTempConfig(t, config)
 }
 
 func (test *cliTest) Execute(args []string) error {
-	// Pass in config file path, if we have one
-	if test.configFile != "" {
+	var hasConfigFlag bool
+	for _, arg := range args {
+		if arg == "--config" {
+			hasConfigFlag = true
+			break
+		}
+	}
+
+	// Use a temporary config file, if none is otherwise set
+	if !hasConfigFlag {
+		// Write an empty config file, if none has been set
+		// we want to avoid accidentally using the system's ~/.dce/config.yml`,
+		// by default
+		if test.configFile == "" {
+			test.WriteConfig(test.t, &configs.Root{})
+		}
+
 		args = append(args, "--config", test.configFile)
 	}
 
@@ -54,6 +70,7 @@ func NewCLITest(t *testing.T) *cliTest {
 	cli := &cliTest{
 		MockPrompter: prompter,
 		stdout:       &stdout,
+		t:            t,
 	}
 
 	// Wrap the `PreRun` method, to inject the mock prompter,

tests/integration/leases_test.go

@@ -0,0 +1,147 @@
+package integration
+
+import (
+	"encoding/json"
+	"github.com/Optum/dce-cli/client/operations"
+	"github.com/Optum/dce-cli/mocks"
+	"github.com/Optum/dce-cli/pkg/service"
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/go-openapi/runtime"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+	"testing"
+	"time"
+)
+
+func TestLeasesCreate(t *testing.T) {
+
+	t.Run("with --expires-on flag", func(t *testing.T) {
+		leasesCreateTest(t, &leaseCreateTestCase{
+			commandArgs: []string{
+				"leases", "create",
+				"-p", "test-user",
+				"-b", "100", "-c", "USD",
+				"-e", "test@example.com",
+				"--expires-on", "1h",
+			},
+			expectedCreateLeaseRequest: operations.PostLeasesBody{
+				BudgetAmount:             aws.Float64(100),
+				BudgetCurrency:           aws.String("USD"),
+				BudgetNotificationEmails: []string{"test@example.com"},
+				ExpiresOn:                float64(time.Now().Add(time.Hour).Unix()),
+				PrincipalID:              aws.String("test-user"),
+			},
+			mockAccountID: "123456789012",
+			expectedJSONOutput: map[string]interface{}{
+				"accountId":                "123456789012",
+				"budgetAmount":             float64(100),
+				"budgetCurrency":           "USD",
+				"budgetNotificationEmails": []interface{}{"test@example.com"},
+				"principalId":              "test-user",
+				"expiresOn":                time.Now().Add(time.Hour).Unix(),
+			},
+		})
+	})
+
+}
+
+type leaseCreateTestCase struct {
+	// Args to pass to the dce-cli
+	commandArgs []string
+
+	// Request body we expect to bbe sent to the `POST /leases` endpoint
+	expectedCreateLeaseRequest operations.PostLeasesBody
+
+	// AccountID to return from the `POST /leases` endpoint
+	mockAccountID string
+
+	// JSON we expect the CLI operation to output to stdout
+	expectedJSONOutput map[string]interface{}
+}
+
+func leasesCreateTest(t *testing.T, testCase *leaseCreateTestCase) {
+	cli := NewCLITest(t)
+
+	// Mock the DCE API
+	api := &mocks.APIer{}
+
+	// Should send a `POST /leases` request
+	expectedLease := testCase.expectedCreateLeaseRequest
+	api.On("PostLeases",
+		mock.MatchedBy(func(params *operations.PostLeasesParams) bool {
+			// Check that ExpiresOn is within ~5s of our expectation
+			// to account for actual time passing
+			assert.InDelta(t, expectedLease.ExpiresOn, params.Lease.ExpiresOn, 5)
+
+			// Pull out ExpiresOn field,
+			// so we can assert.Equals on the rest of the struct
+			expectedLeaseCopy := expectedLease
+			expectedLeaseCopy.ExpiresOn = 0
+			actualLeaseCopy := params.Lease
+			actualLeaseCopy.ExpiresOn = 0
+
+			// Check the rest of the lease object
+			assert.Equal(t, expectedLeaseCopy, actualLeaseCopy)
+			return true
+		}), nil).
+		Return(func(params *operations.PostLeasesParams, wt runtime.ClientAuthInfoWriter) *operations.PostLeasesCreated {
+			// Return a Lease, which looks like the requested lease
+			// (but with an account ID)
+			return &operations.PostLeasesCreated{
+				Payload: &operations.PostLeasesCreatedBody{
+					AccountID:                testCase.mockAccountID,
+					BudgetAmount:             *params.Lease.BudgetAmount,
+					BudgetCurrency:           *params.Lease.BudgetCurrency,
+					BudgetNotificationEmails: params.Lease.BudgetNotificationEmails,
+					ExpiresOn:                params.Lease.ExpiresOn,
+					PrincipalID:              *params.Lease.PrincipalID,
+				},
+			}
+		}, nil)
+
+	// Mock the output writer
+	out := &mocks.OutputWriter{}
+	out.On("Write", mock.MatchedBy(func(out []byte) bool {
+		var actualJSONOutput map[string]interface{}
+		err := json.Unmarshal(out, &actualJSONOutput)
+		assert.Nil(t, err, "output should be valid JSON")
+
+		// Check that ExpiresOn is within ~5s of our expectation
+		// to account for actual time passing
+		_ = testCase.expectedJSONOutput
+		expectedExpiresOn := testCase.expectedJSONOutput["expiresOn"].(int64)
+		actualExpiresOn := actualJSONOutput["expiresOn"].(float64)
+		assert.InDelta(t,
+			expectedExpiresOn,
+			actualExpiresOn,
+			5,
+		)
+
+		// Pull out the expiresOn field,
+		// so we can assert.Equals on the rest of JSON
+		actualJSONOutput["expiresOn"] = int64(0)
+		testCase.expectedJSONOutput["expiresOn"] = int64(0)
+
+		assert.Equal(t, testCase.expectedJSONOutput, actualJSONOutput)
+
+		return true
+	})).Return(0, nil)
+
+	// Mock the Authentication service (would pop open browser to auth user)
+	authSvc := &mocks.Authenticater{}
+	authSvc.On("Authenticate").Return(nil)
+
+	cli.Inject(func(input *injectorInput) {
+		service.ApiClient = api
+		service.Out = out
+		input.service.Authenticater = authSvc
+	})
+
+	// Run `dce leases create` command
+	err := cli.Execute(testCase.commandArgs)
+	require.Nil(t, err)
+
+	api.AssertExpectations(t)
+	out.AssertNumberOfCalls(t, "Write", 1)
+}