Skip to content

Commit c1865a8

Browse files
authored
Merge pull request #78 from eschwartz/fix-expires-on
Fix --expires-on flag parsing
2 parents 9767f8d + e4a2a69 commit c1865a8

File tree

7 files changed

+190
-25
lines changed

7 files changed

+190
-25
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
- Accept YAML configuration and env vars for `dce system deploy` params for namespace and budget notification emails.
99
- Fix bug where modified configuration values used by `dce system deploy` would not be reflected in the terraform deployment
1010
- Fix pre-run credentials check: was accepting expired credentials.
11+
- Fix parsing of `--expires-on` flag for `dce leases create` command
1112

1213
## v0.4.0
1314
- Added `--tf-init-options` and `--tf-apply-options` for greater control over underlying provisioner

pkg/service/accounts.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func (s *AccountsService) AddAccount(accountID, adminRoleARN string) {
2424
},
2525
}
2626
params.SetTimeout(5 * time.Second)
27-
_, err := apiClient.PostAccounts(params, nil)
27+
_, err := ApiClient.PostAccounts(params, nil)
2828
if err != nil {
2929
log.Fatalln("err: ", err)
3030
} else {
@@ -37,7 +37,7 @@ func (s *AccountsService) RemoveAccount(accountID string) {
3737
ID: accountID,
3838
}
3939
params.SetTimeout(5 * time.Second)
40-
_, err := apiClient.DeleteAccountsID(params, nil)
40+
_, err := ApiClient.DeleteAccountsID(params, nil)
4141
if err != nil {
4242
log.Fatalln("err: ", err)
4343
} else {
@@ -50,15 +50,15 @@ func (s *AccountsService) GetAccount(accountID string) {
5050
ID: accountID,
5151
}
5252
params.SetTimeout(5 * time.Second)
53-
res, err := apiClient.GetAccountsID(params, nil)
53+
res, err := ApiClient.GetAccountsID(params, nil)
5454
if err != nil {
5555
log.Fatalln("err: ", err)
5656
}
5757
jsonPayload, err := json.MarshalIndent(res.GetPayload(), "", "\t")
5858
if err != nil {
5959
log.Fatalln("err: ", err)
6060
}
61-
if _, err := out.Write(jsonPayload); err != nil {
61+
if _, err := Out.Write(jsonPayload); err != nil {
6262
log.Fatalln("err: ", err)
6363
}
6464
}
@@ -67,15 +67,15 @@ func (s *AccountsService) GetAccount(accountID string) {
6767
func (s *AccountsService) ListAccounts() {
6868
params := &operations.GetAccountsParams{}
6969
params.SetTimeout(5 * time.Second)
70-
res, err := apiClient.GetAccounts(params, nil)
70+
res, err := ApiClient.GetAccounts(params, nil)
7171
if err != nil {
7272
log.Fatalln("err: ", err)
7373
}
7474
jsonPayload, err := json.MarshalIndent(res.GetPayload(), "", "\t")
7575
if err != nil {
7676
log.Fatalln("err: ", err)
7777
}
78-
if _, err := out.Write(jsonPayload); err != nil {
78+
if _, err := Out.Write(jsonPayload); err != nil {
7979
log.Fatalln("err: ", err)
8080
}
8181
}

pkg/service/leases.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func (s *LeasesService) CreateLease(principalID string, budgetAmount float64, bu
3131

3232
expiry, err := s.Util.ExpandEpochTime(expiresOn)
3333

34-
if err != nil && expiry > 0 {
34+
if err == nil && expiry > 0 {
3535
expiryf := float64(expiry)
3636
postBody.ExpiresOn = expiryf
3737
}
@@ -40,7 +40,7 @@ func (s *LeasesService) CreateLease(principalID string, budgetAmount float64, bu
4040
Lease: postBody,
4141
}
4242
params.SetTimeout(5 * time.Second)
43-
res, err := apiClient.PostLeases(params, nil)
43+
res, err := ApiClient.PostLeases(params, nil)
4444
if err != nil {
4545
log.Fatalln("err: ", err)
4646
}
@@ -50,7 +50,7 @@ func (s *LeasesService) CreateLease(principalID string, budgetAmount float64, bu
5050
}
5151
log.Infoln("Lease created.")
5252

53-
if _, err := out.Write(jsonPayload); err != nil {
53+
if _, err := Out.Write(jsonPayload); err != nil {
5454
log.Fatalln("err: ", err)
5555

5656
}
@@ -64,7 +64,7 @@ func (s *LeasesService) EndLease(accountID, principalID string) {
6464
},
6565
}
6666
params.SetTimeout(5 * time.Second)
67-
_, err := apiClient.DeleteLeases(params, nil)
67+
_, err := ApiClient.DeleteLeases(params, nil)
6868
if err != nil {
6969
log.Fatalln("err: ", err)
7070
}
@@ -76,15 +76,15 @@ func (s *LeasesService) GetLease(leaseID string) {
7676
ID: leaseID,
7777
}
7878
params.SetTimeout(5 * time.Second)
79-
res, err := apiClient.GetLeasesID(params, nil)
79+
res, err := ApiClient.GetLeasesID(params, nil)
8080
if err != nil {
8181
log.Fatalln("err: ", err)
8282
}
8383
jsonPayload, err := json.MarshalIndent(res.GetPayload(), "", "\t")
8484
if err != nil {
8585
log.Fatalln("err: ", err)
8686
}
87-
if _, err := out.Write(jsonPayload); err != nil {
87+
if _, err := Out.Write(jsonPayload); err != nil {
8888
log.Fatalln("err: ", err)
8989
}
9090
}
@@ -99,15 +99,15 @@ func (s *LeasesService) ListLeases(acctID, principalID, nextAcctID, nextPrincipa
9999
Status: &leaseStatus,
100100
}
101101
params.SetTimeout(5 * time.Second)
102-
res, err := apiClient.GetLeases(params, nil)
102+
res, err := ApiClient.GetLeases(params, nil)
103103
if err != nil {
104104
log.Fatalln("err: ", err)
105105
}
106106
jsonPayload, err := json.MarshalIndent(res.GetPayload(), "", "\t")
107107
if err != nil {
108108
log.Fatalln("err: ", err)
109109
}
110-
if _, err := out.Write(jsonPayload); err != nil {
110+
if _, err := Out.Write(jsonPayload); err != nil {
111111
log.Fatalln("err: ", err)
112112
}
113113
}
@@ -125,7 +125,7 @@ func (s *LeasesService) Login(opts *LeaseLoginOptions) {
125125

126126
params := &operations.PostLeasesAuthParams{}
127127
params.SetTimeout(20 * time.Second)
128-
res, err := apiClient.PostLeasesAuth(params, nil)
128+
res, err := ApiClient.PostLeasesAuth(params, nil)
129129

130130
if err != nil {
131131
log.Fatal(err)
@@ -143,7 +143,7 @@ func (s *LeasesService) LoginByID(leaseID string, opts *LeaseLoginOptions) {
143143
ID: leaseID,
144144
}
145145
params.SetTimeout(20 * time.Second)
146-
res, err := apiClient.PostLeasesIDAuth(params, nil)
146+
res, err := ApiClient.PostLeasesIDAuth(params, nil)
147147
if err != nil {
148148
log.Fatalln("err: ", err)
149149
}
@@ -177,7 +177,7 @@ func (s *LeasesService) loginWithCreds(leaseCreds *leaseCreds, opts *LeaseLoginO
177177
leaseCreds.AccessKeyID,
178178
leaseCreds.SecretAccessKey,
179179
leaseCreds.SessionToken)
180-
if _, err := out.Write([]byte(creds)); err != nil {
180+
if _, err := Out.Write([]byte(creds)); err != nil {
181181
log.Fatalln("err: ", err)
182182
}
183183
}

pkg/service/service.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,15 @@ type ServiceContainer struct {
2020
}
2121

2222
var log observ.Logger
23-
var apiClient utl.APIer
24-
var out observ.OutputWriter
23+
var ApiClient utl.APIer
24+
var Out observ.OutputWriter
2525

2626
// New returns a new ServiceContainer given config
2727
func New(config *configs.Root, observation *observ.ObservationContainer, util *utl.UtilContainer) *ServiceContainer {
2828

2929
log = observation.Logger
30-
apiClient = util.APIer
31-
out = observation.OutputWriter
30+
ApiClient = util.APIer
31+
Out = observation.OutputWriter
3232

3333
serviceContainer := ServiceContainer{
3434
Config: config,

pkg/service/usage.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,15 @@ func (s *UsageService) GetUsage(startDate, endDate float64) {
2222
EndDate: endDate,
2323
}
2424
params.SetTimeout(5 * time.Second)
25-
res, err := apiClient.GetUsage(params, nil)
25+
res, err := ApiClient.GetUsage(params, nil)
2626
if err != nil {
2727
log.Fatalln("err: ", err)
2828
} else {
2929
jsonPayload, err := json.MarshalIndent(res.GetPayload(), "", "\t")
3030
if err != nil {
3131
log.Fatalln("err: ", err)
3232
}
33-
if _, err := out.Write(jsonPayload); err != nil {
33+
if _, err := Out.Write(jsonPayload); err != nil {
3434
log.Fatalln("err: ", err)
3535
}
3636
}

tests/integration/cli_test_util.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,31 @@ type cliTest struct {
2323
stdout *bytes.Buffer
2424
injector injector
2525
configFile string
26+
t *testing.T
2627
}
2728

2829
func (test *cliTest) WriteConfig(t *testing.T, config *configs.Root) {
2930
test.configFile = writeTempConfig(t, config)
3031
}
3132

3233
func (test *cliTest) Execute(args []string) error {
33-
// Pass in config file path, if we have one
34-
if test.configFile != "" {
34+
var hasConfigFlag bool
35+
for _, arg := range args {
36+
if arg == "--config" {
37+
hasConfigFlag = true
38+
break
39+
}
40+
}
41+
42+
// Use a temporary config file, if none is otherwise set
43+
if !hasConfigFlag {
44+
// Write an empty config file, if none has been set
45+
// we want to avoid accidentally using the system's ~/.dce/config.yml`,
46+
// by default
47+
if test.configFile == "" {
48+
test.WriteConfig(test.t, &configs.Root{})
49+
}
50+
3551
args = append(args, "--config", test.configFile)
3652
}
3753

@@ -54,6 +70,7 @@ func NewCLITest(t *testing.T) *cliTest {
5470
cli := &cliTest{
5571
MockPrompter: prompter,
5672
stdout: &stdout,
73+
t: t,
5774
}
5875

5976
// Wrap the `PreRun` method, to inject the mock prompter,

tests/integration/leases_test.go

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
package integration
2+
3+
import (
4+
"encoding/json"
5+
"github.com/Optum/dce-cli/client/operations"
6+
"github.com/Optum/dce-cli/mocks"
7+
"github.com/Optum/dce-cli/pkg/service"
8+
"github.com/aws/aws-sdk-go/aws"
9+
"github.com/go-openapi/runtime"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/mock"
12+
"github.com/stretchr/testify/require"
13+
"testing"
14+
"time"
15+
)
16+
17+
func TestLeasesCreate(t *testing.T) {
18+
19+
t.Run("with --expires-on flag", func(t *testing.T) {
20+
leasesCreateTest(t, &leaseCreateTestCase{
21+
commandArgs: []string{
22+
"leases", "create",
23+
"-p", "test-user",
24+
"-b", "100", "-c", "USD",
25+
26+
"--expires-on", "1h",
27+
},
28+
expectedCreateLeaseRequest: operations.PostLeasesBody{
29+
BudgetAmount: aws.Float64(100),
30+
BudgetCurrency: aws.String("USD"),
31+
BudgetNotificationEmails: []string{"[email protected]"},
32+
ExpiresOn: float64(time.Now().Add(time.Hour).Unix()),
33+
PrincipalID: aws.String("test-user"),
34+
},
35+
mockAccountID: "123456789012",
36+
expectedJSONOutput: map[string]interface{}{
37+
"accountId": "123456789012",
38+
"budgetAmount": float64(100),
39+
"budgetCurrency": "USD",
40+
"budgetNotificationEmails": []interface{}{"[email protected]"},
41+
"principalId": "test-user",
42+
"expiresOn": time.Now().Add(time.Hour).Unix(),
43+
},
44+
})
45+
})
46+
47+
}
48+
49+
type leaseCreateTestCase struct {
50+
// Args to pass to the dce-cli
51+
commandArgs []string
52+
53+
// Request body we expect to bbe sent to the `POST /leases` endpoint
54+
expectedCreateLeaseRequest operations.PostLeasesBody
55+
56+
// AccountID to return from the `POST /leases` endpoint
57+
mockAccountID string
58+
59+
// JSON we expect the CLI operation to output to stdout
60+
expectedJSONOutput map[string]interface{}
61+
}
62+
63+
func leasesCreateTest(t *testing.T, testCase *leaseCreateTestCase) {
64+
cli := NewCLITest(t)
65+
66+
// Mock the DCE API
67+
api := &mocks.APIer{}
68+
69+
// Should send a `POST /leases` request
70+
expectedLease := testCase.expectedCreateLeaseRequest
71+
api.On("PostLeases",
72+
mock.MatchedBy(func(params *operations.PostLeasesParams) bool {
73+
// Check that ExpiresOn is within ~5s of our expectation
74+
// to account for actual time passing
75+
assert.InDelta(t, expectedLease.ExpiresOn, params.Lease.ExpiresOn, 5)
76+
77+
// Pull out ExpiresOn field,
78+
// so we can assert.Equals on the rest of the struct
79+
expectedLeaseCopy := expectedLease
80+
expectedLeaseCopy.ExpiresOn = 0
81+
actualLeaseCopy := params.Lease
82+
actualLeaseCopy.ExpiresOn = 0
83+
84+
// Check the rest of the lease object
85+
assert.Equal(t, expectedLeaseCopy, actualLeaseCopy)
86+
return true
87+
}), nil).
88+
Return(func(params *operations.PostLeasesParams, wt runtime.ClientAuthInfoWriter) *operations.PostLeasesCreated {
89+
// Return a Lease, which looks like the requested lease
90+
// (but with an account ID)
91+
return &operations.PostLeasesCreated{
92+
Payload: &operations.PostLeasesCreatedBody{
93+
AccountID: testCase.mockAccountID,
94+
BudgetAmount: *params.Lease.BudgetAmount,
95+
BudgetCurrency: *params.Lease.BudgetCurrency,
96+
BudgetNotificationEmails: params.Lease.BudgetNotificationEmails,
97+
ExpiresOn: params.Lease.ExpiresOn,
98+
PrincipalID: *params.Lease.PrincipalID,
99+
},
100+
}
101+
}, nil)
102+
103+
// Mock the output writer
104+
out := &mocks.OutputWriter{}
105+
out.On("Write", mock.MatchedBy(func(out []byte) bool {
106+
var actualJSONOutput map[string]interface{}
107+
err := json.Unmarshal(out, &actualJSONOutput)
108+
assert.Nil(t, err, "output should be valid JSON")
109+
110+
// Check that ExpiresOn is within ~5s of our expectation
111+
// to account for actual time passing
112+
_ = testCase.expectedJSONOutput
113+
expectedExpiresOn := testCase.expectedJSONOutput["expiresOn"].(int64)
114+
actualExpiresOn := actualJSONOutput["expiresOn"].(float64)
115+
assert.InDelta(t,
116+
expectedExpiresOn,
117+
actualExpiresOn,
118+
5,
119+
)
120+
121+
// Pull out the expiresOn field,
122+
// so we can assert.Equals on the rest of JSON
123+
actualJSONOutput["expiresOn"] = int64(0)
124+
testCase.expectedJSONOutput["expiresOn"] = int64(0)
125+
126+
assert.Equal(t, testCase.expectedJSONOutput, actualJSONOutput)
127+
128+
return true
129+
})).Return(0, nil)
130+
131+
// Mock the Authentication service (would pop open browser to auth user)
132+
authSvc := &mocks.Authenticater{}
133+
authSvc.On("Authenticate").Return(nil)
134+
135+
cli.Inject(func(input *injectorInput) {
136+
service.ApiClient = api
137+
service.Out = out
138+
input.service.Authenticater = authSvc
139+
})
140+
141+
// Run `dce leases create` command
142+
err := cli.Execute(testCase.commandArgs)
143+
require.Nil(t, err)
144+
145+
api.AssertExpectations(t)
146+
out.AssertNumberOfCalls(t, "Write", 1)
147+
}

0 commit comments

Comments
 (0)