Skip to content

Commit 3373ef9

Browse files
t0yv0justinvpZaid-AjajVenelinMartinov
authored
Implicit --refresh (#374)
* t0yv0/pulumi-v3.175.0 * Implicit --refresh * Lint * TestRefreshImplicitly * This TODO is done * TestRefresh should pass now * TestRefreshDeleted now working * TestRefreshNoChanges working now * TestRefreshImplicitly now tested on both executors * Skip the test causing engine integrity error * Remove drift from test delete plan * Remove more drift from test plans * x-link the TODO * Fix TestE2eTs * Fix Test_E2e_DotNet * Fix Test_RdsExample * Fix Test_E2e_*Lang* * Fix Test_EksExample * Fix Test_AlbExample * Fix pointer expectation one-off * Unskip Test_replace_drift_deleted (#401) * t0yv0/pulumi-v3.175.0 (#373) * Better missing credentials error when using AWS modules (#375) * Better missing credentials error when using AWS modules * embed error message and link to configuring explicit providers * Update README for executor changes. (#388) * non-nullable inputs should be marked as required (#385) * non-nullable inputs should be marked as required * add opentofu to the executors of the test * Fix VPC module typings, include outputs that refer to serialized resources (#393) * Default output type is any, better inference for try(...) expressions (#398) * Unskip Test_replace_drift_deleted The issue in the CLI has been addressed and released, so we can unskip this test. * dependencies: pulumi v3.176.0 (#402) * dependencies: pulumi v3.176.0 Update Pulumi to v3.176.0. With this version, the feature flag no longer needs to be set; views are enabled by default. * Set `PULUMI_ENABLE_VIEWS_PREVIEW` for YAML We need to release a new version of pulumi-yaml, pull that new version into pulumi/pulumi, and then release a new version of pulumi/pulumi to avoid the need to set the envvar for YAML programs. --------- Co-authored-by: Anton Tayanovskyy <[email protected]> Co-authored-by: Zaid Ajaj <[email protected]> Co-authored-by: VenelinMartinov <[email protected]> Co-authored-by: Zaid Ajaj <[email protected]> * make it compile * actually make it compile * Fix TestE2eTs * Show installed pulumi version * try removing drift detection hack * try removing drift detection hack #2 * lint * remove drift detection from module handler and expect no changes/drift in tests * remove --refresh from preview in RDS example --------- Co-authored-by: Justin Van Patten <[email protected]> Co-authored-by: Zaid Ajaj <[email protected]> Co-authored-by: VenelinMartinov <[email protected]> Co-authored-by: Zaid Ajaj <[email protected]>
1 parent 4630cae commit 3373ef9

18 files changed

+499
-1103
lines changed

.github/actions/setup-tools/action.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,8 @@ runs:
9797
shell: bash
9898
run: |
9999
terraform -version
100+
101+
- name: Show Pulumi Version
102+
shell: bash
103+
run: |
104+
pulumi version

pkg/modprovider/module.go

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ func (h *moduleHandler) Diff(
7979
inferredModule *InferredModuleSchema,
8080
executor string,
8181
) (*pulumirpc.DiffResponse, error) {
82+
urn := urn.URN(req.GetUrn())
83+
8284
oldInputs, err := plugin.UnmarshalProperties(req.GetOldInputs(), h.marshalOpts())
8385
if err != nil {
8486
return nil, err
@@ -89,15 +91,13 @@ func (h *moduleHandler) Diff(
8991
return nil, err
9092
}
9193

92-
// TODO[pulumi/pulumi-terraform-module#332] detect and correct drift
9394
if !oldInputs.DeepEquals(newInputs) {
9495
// Inputs have changed, so we need tell the engine that an update is needed.
9596
return &pulumirpc.DiffResponse{Changes: pulumirpc.DiffResponse_DIFF_SOME}, nil
9697
}
9798

9899
// Here, inputs have not changes but the underlying module might have changed
99100
// perform a plan to see if there were any changes in the module reported by terraform
100-
urn := urn.URN(req.GetUrn())
101101
oldOutputs, err := plugin.UnmarshalProperties(req.GetOlds(), h.marshalOpts())
102102
if err != nil {
103103
return nil, fmt.Errorf("failed to unmarshal old outputs: %w", err)
@@ -233,9 +233,11 @@ func (h *moduleHandler) applyModuleOperation(
233233

234234
logger := newResourceLogger(h.hc, urn)
235235

236+
// Because of RefreshBeforeUpdate, Pulumi CLI has already refreshed at this point.
237+
// so we use plan -refresh=false via tfsandbox.PlanNoRefresh()
236238
// Plans are always needed, so this code will run in DryRun and otherwise. In the future we
237239
// may be able to reuse the plan from DryRun for the subsequent application.
238-
plan, err := tf.Plan(ctx, logger)
240+
plan, err := tf.PlanNoRefresh(ctx, logger)
239241
if err != nil {
240242
return nil, nil, fmt.Errorf("Plan failed: %w", err)
241243
}
@@ -251,7 +253,10 @@ func (h *moduleHandler) applyModuleOperation(
251253
views = viewStepsPlan(packageName, plan)
252254
moduleOutputs = plan.Outputs()
253255
} else {
254-
tfState, err := tf.Apply(ctx, logger) // TODO[pulumi/pulumi-terraform-module#341] reuse the plan
256+
// TODO[pulumi/pulumi-terraform-module#341] reuse the plan
257+
tfState, err := tf.Apply(ctx, logger, tfsandbox.RefreshOpts{
258+
NoRefresh: true, // we already refreshed before this point
259+
})
255260
if tfState != nil {
256261
msg := fmt.Sprintf("tf.Apply produced the following state: %s", tfState.PrettyPrint())
257262
logger.Log(ctx, tfsandbox.Debug, msg)
@@ -380,8 +385,9 @@ func (h *moduleHandler) Create(
380385
contract.AssertNoErrorf(err, "plugin.MarshalProperties should not fail")
381386

382387
return &pulumirpc.CreateResponse{
383-
Id: moduleStateResourceID,
384-
Properties: props,
388+
Id: moduleStateResourceID,
389+
Properties: props,
390+
RefreshBeforeUpdate: true,
385391
}, nil
386392
}
387393

@@ -447,7 +453,8 @@ func (h *moduleHandler) Update(
447453
contract.AssertNoErrorf(err, "plugin.MarshalProperties should not fail")
448454

449455
return &pulumirpc.UpdateResponse{
450-
Properties: props,
456+
Properties: props,
457+
RefreshBeforeUpdate: true,
451458
}, nil
452459
}
453460

@@ -575,7 +582,7 @@ func (h *moduleHandler) Read(
575582
executor,
576583
)
577584
if err != nil {
578-
return nil, fmt.Errorf("Failed preparing tofu sandbox: %w", err)
585+
return nil, fmt.Errorf("failed preparing tofu sandbox: %w", err)
579586
}
580587

581588
plan, err := tf.PlanRefreshOnly(ctx, logger)
@@ -587,7 +594,7 @@ func (h *moduleHandler) Read(
587594
state, err := tf.Refresh(ctx, logger)
588595
if err != nil {
589596
logger.Log(ctx, tfsandbox.Debug, fmt.Sprintf("error running refresh: %v", err))
590-
return nil, fmt.Errorf("Module refresh failed: %w", err)
597+
return nil, fmt.Errorf("module refresh failed: %w", err)
591598
}
592599

593600
outputs, err := h.outputs(ctx, tf, state)
@@ -613,10 +620,19 @@ func (h *moduleHandler) Read(
613620
return nil, err
614621
}
615622

623+
// inputs never change on refresh
624+
freshInputs := moduleInputs
625+
626+
freshInputsStruct, err := plugin.MarshalProperties(freshInputs, h.marshalOpts())
627+
if err != nil {
628+
return nil, err
629+
}
630+
616631
return &pulumirpc.ReadResponse{
617-
Id: moduleResourceID,
618-
Properties: properties,
619-
Inputs: req.GetInputs(), // inputs never change on refresh
632+
Id: moduleResourceID,
633+
Properties: properties,
634+
Inputs: freshInputsStruct,
635+
RefreshBeforeUpdate: true,
620636
}, nil
621637
}
622638

pkg/modprovider/views.go

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ func viewStepsGeneric(
5757
preview bool,
5858
) []*pulumirpc.ViewStep {
5959
var steps []*pulumirpc.ViewStep
60-
priorState, hasPriorState := plan.PriorState()
6160
hasFinalState := finalState != nil
6261

6362
counter := 0
@@ -73,14 +72,7 @@ func viewStepsGeneric(
7372
// TODO[pulumi/pulumi-terraform-module#61] sometimes addresses change but identity remains the same.
7473
addr := rplan.Address()
7574

76-
var priorRState, finalRState *tfsandbox.ResourceState
77-
78-
if hasPriorState {
79-
s, ok := priorState.FindResourceState(addr)
80-
if ok {
81-
priorRState = s
82-
}
83-
}
75+
var finalRState *tfsandbox.ResourceState
8476

8577
if hasFinalState {
8678
s, ok := finalState.FindResourceState(addr)
@@ -89,7 +81,7 @@ func viewStepsGeneric(
8981
}
9082
}
9183

92-
rSteps := viewStepsForResource(packageName, rplan, priorRState, finalRState, preview)
84+
rSteps := viewStepsForResource(packageName, rplan, finalRState, preview)
9385
steps = append(steps, rSteps...)
9486
})
9587

@@ -121,11 +113,21 @@ func viewStepsGeneric(
121113
return steps
122114
}
123115

124-
func viewStepOp(changeKind tfsandbox.ChangeKind) []pulumirpc.ViewStep_Op {
116+
func viewStepOp(changeKind tfsandbox.ChangeKind, _ bool /*drift*/) []pulumirpc.ViewStep_Op {
125117
switch changeKind {
126118
case tfsandbox.NoOp:
127119
return []pulumirpc.ViewStep_Op{pulumirpc.ViewStep_SAME}
128120
case tfsandbox.Update:
121+
// TODO this does not seem to work, per Justin:
122+
//
123+
// will not work with the current implementation… If you sent an Op UPDATE for the view, I think it will.
124+
//
125+
//
126+
// Need to figure out if this is temporary or final.
127+
//
128+
// if drift {
129+
// return []pulumirpc.ViewStep_Op{pulumirpc.ViewStep_REFRESH}
130+
// }
129131
return []pulumirpc.ViewStep_Op{pulumirpc.ViewStep_UPDATE}
130132
case tfsandbox.Replace:
131133
return []pulumirpc.ViewStep_Op{
@@ -141,8 +143,6 @@ func viewStepOp(changeKind tfsandbox.ChangeKind) []pulumirpc.ViewStep_Op {
141143
}
142144
case tfsandbox.Create:
143145
return []pulumirpc.ViewStep_Op{pulumirpc.ViewStep_CREATE}
144-
case tfsandbox.Read:
145-
return []pulumirpc.ViewStep_Op{pulumirpc.ViewStep_REFRESH}
146146
case tfsandbox.Delete:
147147
return []pulumirpc.ViewStep_Op{pulumirpc.ViewStep_DELETE}
148148
case tfsandbox.Forget:
@@ -211,7 +211,6 @@ func viewStepForSameResource(
211211
func viewStepsForResource(
212212
packageName packageName,
213213
rplan ResourcePlan,
214-
priorState ResourceState, // may be nil in operations such as create
215214
finalState ResourceState, // may be nil when planning or failed to create
216215
preview bool,
217216
) []*pulumirpc.ViewStep {
@@ -221,7 +220,7 @@ func viewStepsForResource(
221220
ty := childResourceTypeToken(packageName, rplan.Type()).String()
222221
name := childResourceName(addr)
223222

224-
var oldViewState, newViewState *pulumirpc.ViewStepState
223+
var newViewState *pulumirpc.ViewStepState
225224
if finalState != nil {
226225
newViewState = viewStepState(packageName, addr, tfType, finalState.AttributeValues())
227226
} else {
@@ -231,13 +230,15 @@ func viewStepsForResource(
231230
}
232231
}
233232

234-
if priorState != nil {
235-
oldViewState = viewStepState(packageName, addr, tfType, priorState.AttributeValues())
233+
var oldViewState *pulumirpc.ViewStepState
234+
before, hasBefore := rplan.Before()
235+
if hasBefore {
236+
oldViewState = viewStepState(packageName, addr, tfType, before)
236237
}
237238

238239
steps := []*pulumirpc.ViewStep{}
239240

240-
for _, op := range viewStepOp(rplan.ChangeKind()) {
241+
for _, op := range viewStepOp(rplan.ChangeKind(), rplan.Drift()) {
241242
newViewStateToSend := newViewState
242243
if op == pulumirpc.ViewStep_DELETE_REPLACED {
243244
newViewStateToSend = nil

pkg/tfsandbox/apply.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import (
1313
// Apply can return both a non-nil State and a non-nil error. If the apply
1414
// fails, but some resources were created and written to the TF State we will return
1515
// the state and the apply error.
16-
func (t *ModuleRuntime) Apply(ctx context.Context, logger Logger) (*State, error) {
17-
state, applyErr := t.apply(ctx, logger)
16+
func (t *ModuleRuntime) Apply(ctx context.Context, logger Logger, opts RefreshOpts) (*State, error) {
17+
state, applyErr := t.apply(ctx, logger, opts)
1818
s, err := NewState(state)
1919
if err != nil {
2020
return nil, err
@@ -23,11 +23,20 @@ func (t *ModuleRuntime) Apply(ctx context.Context, logger Logger) (*State, error
2323
}
2424

2525
// Apply runs the terraform apply command and returns the final state
26-
func (t *ModuleRuntime) apply(ctx context.Context, logger Logger) (*tfjson.State, error) {
26+
func (t *ModuleRuntime) apply(ctx context.Context, logger Logger, opts RefreshOpts) (*tfjson.State, error) {
2727
logWriter := newJSONLogPipe(ctx, logger)
2828
defer logWriter.Close()
2929

30-
applyErr := t.tf.ApplyJSON(ctx, logWriter, t.applyOptions()...)
30+
aOpts := []tfexec.ApplyOption{}
31+
32+
if opts.NoRefresh {
33+
aOpts = append(aOpts, tfexec.Refresh(false))
34+
}
35+
if opts.RefreshOnly {
36+
aOpts = append(aOpts, tfexec.RefreshOnly(true))
37+
}
38+
39+
applyErr := t.tf.ApplyJSON(ctx, logWriter, t.applyOptions(aOpts...)...)
3140
// if the apply failed just log it to debug logs and continue
3241
// we want to return and process the partial state from a failed apply
3342
if applyErr != nil {

pkg/tfsandbox/details.go

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@ const (
3636
Replace
3737
ReplaceDestroyBeforeCreate
3838
Create
39-
Read
4039
Delete
4140

41+
// Need to pin down when Read is used, might not be in use; possibly only for data sources.
42+
Read
43+
4244
// Need to pin down when Forget operations arise.
4345
// Likely during https://developer.hashicorp.com/terraform/cli/commands/state/rm
4446
// Roughly but possibly not exactly equivalent to
@@ -51,6 +53,12 @@ type ResourcePlan struct {
5153
resourceChange *tfjson.ResourceChange
5254

5355
plannedState *tfjson.StateResource // may be nil when planning removal
56+
57+
drift bool // when true, the change comes from ResourceDrift, e.g. refresh
58+
}
59+
60+
func (p *ResourcePlan) Drift() bool {
61+
return p.drift
5462
}
5563

5664
// TODO sometimes address changes while identity remains the same, e.g see PreviousAddress. The call sites need to be
@@ -73,6 +81,31 @@ func (p *ResourcePlan) PlannedValues() (resource.PropertyMap, bool) {
7381
return extractPropertyMapFromPlan(*p.plannedState, p.resourceChange), true
7482
}
7583

84+
// The values before the change. For some change such as creating the resource these values are not available. Note
85+
// also that for some changes such as refreshing a resource that has drifted, the Before values will be different from
86+
// values found in the resource's prior state. The Before() values will be the fresh values detected by Read.
87+
func (p *ResourcePlan) Before() (resource.PropertyMap, bool) {
88+
change := p.resourceChange.Change
89+
if change == nil {
90+
return nil, false
91+
}
92+
before := change.Before
93+
if before == nil {
94+
return nil, false
95+
}
96+
beforeMap, ok := before.(map[string]any)
97+
if !ok {
98+
return nil, false
99+
}
100+
pm := extractPropertyMapFromAttributeValues(beforeMap)
101+
if change.BeforeSensitive != nil {
102+
obj := resource.NewObjectProperty(pm)
103+
upd := updateResourceValue(obj, change.BeforeSensitive, resourceMakeSecretConservative)
104+
pm = upd.ObjectValue()
105+
}
106+
return pm, true
107+
}
108+
76109
// Describes what change is being planned.
77110
func (p *ResourcePlan) ChangeKind() ChangeKind {
78111
contract.Assertf(p.resourceChange != nil, "cannot determine ChangeKind")
@@ -115,8 +148,13 @@ func (s *ResourceState) AttributeValues() resource.PropertyMap {
115148
type Plan struct {
116149
rawPlan *tfjson.Plan
117150
byAddress map[ResourceAddress]*ResourcePlan
151+
hasDrift bool
118152
}
119153

154+
//func (p *Plan) HasDrift() bool {
155+
// return p.hasDrift
156+
//}
157+
120158
func (p *Plan) VisitResourcePlans(visitor func(*ResourcePlan)) {
121159
for _, rp := range p.byAddress {
122160
visitor(rp)
@@ -134,15 +172,29 @@ func NewPlan(rawPlan *tfjson.Plan) (*Plan, error) {
134172
return nil, fmt.Errorf("unexpected error extracting planned values from *tfjson.Plan: %w", err)
135173
}
136174

137-
p := &Plan{rawPlan: rawPlan, byAddress: map[ResourceAddress]*ResourcePlan{}}
175+
if len(rawPlan.ResourceDrift) > 0 && len(rawPlan.ResourceChanges) > 0 {
176+
contract.Failf("Currently cannot handle plans with both resource_drift and resource_changes.\n" +
177+
"Make sure plans are obtained with -refresh-only (no changes) or -refresh=false (no drift)")
178+
}
179+
180+
drift := len(rawPlan.ResourceDrift) > 0
181+
182+
p := &Plan{
183+
rawPlan: rawPlan,
184+
byAddress: map[ResourceAddress]*ResourcePlan{},
185+
hasDrift: drift,
186+
}
138187

139-
for _, ch := range rawPlan.ResourceChanges {
188+
for _, ch := range append(rawPlan.ResourceChanges, rawPlan.ResourceDrift...) {
140189
// Exclude entries pertaining to data source look-ups, only interested in resources proper.
141190
if ch.Mode == tfjson.DataResourceMode {
142191
continue
143192
}
144193

145-
plan := &ResourcePlan{resourceChange: ch}
194+
plan := &ResourcePlan{
195+
resourceChange: ch,
196+
drift: drift,
197+
}
146198
addr := ResourceAddress(ch.Address)
147199
plannedState, ok := resourcePlannedValues[addr]
148200
if ok {
@@ -191,15 +243,6 @@ func (p *Plan) Outputs() resource.PropertyMap {
191243
return outputs
192244
}
193245

194-
func (p *Plan) PriorState() (*State, bool) {
195-
if p.rawPlan.PriorState == nil {
196-
return nil, false
197-
}
198-
st, err := NewState(p.rawPlan.PriorState)
199-
contract.AssertNoErrorf(err, "newState failed when processing PriorState")
200-
return st, true
201-
}
202-
203246
// RawPlan returns the raw tfjson.Plan
204247
// NOTE: this is exposed for testing purposes only
205248
func (p *Plan) RawPlan() *tfjson.Plan {

0 commit comments

Comments
 (0)