Skip to content

Commit

Permalink
app/agent.go: remove token from state file
Browse files Browse the repository at this point in the history
  • Loading branch information
tommyhahn committed Nov 29, 2018
1 parent 25c2208 commit d90140a
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 37 deletions.
7 changes: 3 additions & 4 deletions agent/app/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func (agent *ecsAgent) doStart(containerChangeEventStream *eventstream.EventStre

// Initialize the state manager
stateManager, err := agent.newStateManager(taskEngine,
&agent.cfg.Cluster, &agent.containerInstanceARN, &currentEC2InstanceID, &agent.availabilityZone, &agent.registrationToken)
&agent.cfg.Cluster, &agent.containerInstanceARN, &currentEC2InstanceID, &agent.availabilityZone)
if err != nil {
seelog.Criticalf("Error creating state manager: %v", err)
return exitcodes.ExitTerminal
Expand Down Expand Up @@ -333,7 +333,7 @@ func (agent *ecsAgent) newTaskEngine(containerChangeEventStream *eventstream.Eve
// previousStateManager is used to verify that our current runtime configuration is
// compatible with our past configuration as reflected by our state-file
previousStateManager, err := agent.newStateManager(previousTaskEngine, &previousCluster,
&previousContainerInstanceArn, &previousEC2InstanceID, &previousAZ, &previousRegistrationToken)
&previousContainerInstanceArn, &previousEC2InstanceID, &previousAZ)
if err != nil {
seelog.Criticalf("Error creating state manager: %v", err)
return nil, "", err
Expand Down Expand Up @@ -419,7 +419,7 @@ func (agent *ecsAgent) newStateManager(
cluster *string,
containerInstanceArn *string,
savedInstanceID *string,
availabilityZone *string, registrationToken *string) (statemanager.StateManager, error) {
availabilityZone *string) (statemanager.StateManager, error) {

if !agent.cfg.Checkpoint {
return statemanager.NewNoopStateManager(), nil
Expand All @@ -434,7 +434,6 @@ func (agent *ecsAgent) newStateManager(
// This is for making testing easier as we can mock this
agent.saveableOptionFactory.AddSaveable("EC2InstanceID", savedInstanceID),
agent.saveableOptionFactory.AddSaveable("availabilityZone", availabilityZone),
agent.saveableOptionFactory.AddSaveable("RegistrationToken", registrationToken),
)
}

Expand Down
6 changes: 3 additions & 3 deletions agent/app/agent_compatibility_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestCompatibilityEnabledSuccess(t *testing.T) {

gomock.InOrder(
saveableOptionFactory.EXPECT().AddSaveable(gomock.Any(), gomock.Any()).AnyTimes(),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(stateManager, nil),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(stateManager, nil),
stateManager.EXPECT().Load().AnyTimes(),
state.EXPECT().AllTasks().Return([]*apitask.Task{}),
)
Expand Down Expand Up @@ -79,7 +79,7 @@ func TestCompatibilityDefaultEnabledFail(t *testing.T) {
}
gomock.InOrder(
saveableOptionFactory.EXPECT().AddSaveable(gomock.Any(), gomock.Any()).AnyTimes(),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(stateManager, nil),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(stateManager, nil),
stateManager.EXPECT().Load().AnyTimes(),
state.EXPECT().AllTasks().Return(getTaskListWithOneBadTask()),
)
Expand Down Expand Up @@ -110,7 +110,7 @@ func TestCompatibilityExplicitlyEnabledFail(t *testing.T) {
}
gomock.InOrder(
saveableOptionFactory.EXPECT().AddSaveable(gomock.Any(), gomock.Any()).AnyTimes(),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(stateManager, nil),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(stateManager, nil),
stateManager.EXPECT().Load().AnyTimes(),
state.EXPECT().AllTasks().Return(getTaskListWithOneBadTask()),
)
Expand Down
40 changes: 10 additions & 30 deletions agent/app/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,10 @@ func TestDoStartNewTaskEngineError(t *testing.T) {
saveableOptionFactory.EXPECT().AddSaveable("Cluster", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("EC2InstanceID", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("availabilityZone", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("RegistrationToken", gomock.Any()).Return(nil),

// An error in creating the state manager should result in an
// error from newTaskEngine as well
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
).Return(
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(
nil, errors.New("error")),
)

Expand Down Expand Up @@ -191,8 +189,7 @@ func TestDoStartNewStateManagerError(t *testing.T) {
saveableOptionFactory.EXPECT().AddSaveable("Cluster", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("EC2InstanceID", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("availabilityZone", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("RegistrationToken", gomock.Any()).Return(nil),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(
statemanager.NewNoopStateManager(), nil),
state.EXPECT().AllTasks().AnyTimes(),
Expand All @@ -201,8 +198,7 @@ func TestDoStartNewStateManagerError(t *testing.T) {
saveableOptionFactory.EXPECT().AddSaveable("Cluster", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("EC2InstanceID", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("availabilityZone", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("RegistrationToken", gomock.Any()).Return(nil),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(
nil, errors.New("error")),
)
Expand Down Expand Up @@ -385,13 +381,7 @@ func TestNewTaskEngineRestoreFromCheckpointNoEC2InstanceIDToLoadHappyPath(t *tes
saveableOptionFactory.EXPECT().AddSaveable("Cluster", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("EC2InstanceID", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("availabilityZone", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("RegistrationToken", gomock.Any()).Do(
func(name string, saveable statemanager.Saveable) {
previousRegistrationToken, ok := saveable.(*string)
assert.True(t, ok)
*previousRegistrationToken = "prev-reg-token"
}).Return(nil),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(
statemanager.NewNoopStateManager(), nil),
state.EXPECT().AllTasks().AnyTimes(),
Expand Down Expand Up @@ -448,13 +438,7 @@ func TestNewTaskEngineRestoreFromCheckpointPreviousEC2InstanceIDLoadedHappyPath(
assert.True(t, ok)
*previousAZ = "us-west-2b"
}).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("RegistrationToken", gomock.Any()).Do(
func(name string, saveable statemanager.Saveable) {
previousRegistrationToken, ok := saveable.(*string)
assert.True(t, ok)
*previousRegistrationToken = "prev-reg-token"
}).Return(nil),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(
statemanager.NewNoopStateManager(), nil),
state.EXPECT().AllTasks().AnyTimes(),
Expand Down Expand Up @@ -509,9 +493,8 @@ func TestNewTaskEngineRestoreFromCheckpointClusterIDMismatch(t *testing.T) {
}).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("EC2InstanceID", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("availabilityZone", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("RegistrationToken", gomock.Any()).Return(nil),

stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(
statemanager.NewNoopStateManager(), nil),
state.EXPECT().AllTasks().AnyTimes(),
Expand Down Expand Up @@ -548,8 +531,7 @@ func TestNewTaskEngineRestoreFromCheckpointNewStateManagerError(t *testing.T) {
saveableOptionFactory.EXPECT().AddSaveable("Cluster", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("EC2InstanceID", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("availabilityZone", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("RegistrationToken", gomock.Any()).Return(nil),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(), gomock.Any(),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(
nil, errors.New("error")),
)
Expand Down Expand Up @@ -584,9 +566,8 @@ func TestNewTaskEngineRestoreFromCheckpointStateLoadError(t *testing.T) {
saveableOptionFactory.EXPECT().AddSaveable("Cluster", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("EC2InstanceID", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("availabilityZone", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("RegistrationToken", gomock.Any()).Return(nil),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
).Return(stateManager, nil),
stateManager.EXPECT().Load().Return(errors.New("error")),
)
Expand Down Expand Up @@ -622,9 +603,8 @@ func TestNewTaskEngineRestoreFromCheckpoint(t *testing.T) {
saveableOptionFactory.EXPECT().AddSaveable("Cluster", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("EC2InstanceID", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("availabilityZone", gomock.Any()).Return(nil),
saveableOptionFactory.EXPECT().AddSaveable("RegistrationToken", gomock.Any()).Return(nil),
stateManagerFactory.EXPECT().NewStateManager(gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
).Return(statemanager.NewNoopStateManager(), nil),
state.EXPECT().AllTasks().AnyTimes(),
ec2MetadataClient.EXPECT().InstanceID().Return(expectedInstanceID, nil),
Expand Down

0 comments on commit d90140a

Please sign in to comment.