Skip to content

Commit

Permalink
Merge #11010
Browse files Browse the repository at this point in the history
11010: Remove warnings about component options from diagnostics r=AaronFriel a=Frassle

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

This still writes a message to the V10 log stream, but that by default doesn't show up for most users.

I've removed the test because I don't think there's any sensible way to hook into glog's global state to test this (That's two PRs in two days that make me miss
[ILogger](https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.logging.ilogger)).

## Checklist

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [x] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


Co-authored-by: Fraser Waters <fraser@pulumi.com>
  • Loading branch information
bors[bot] and Frassle committed Oct 13, 2022
2 parents 9ba644b + 9a62667 commit f713611
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 96 deletions.
@@ -0,0 +1,4 @@
changes:
- type: fix
scope: cli/display,engine
description: Use of unsupported ResourceOptions on components will no longer raise resource warnings, instead they are just logged to the diagnostic error stream.
91 changes: 0 additions & 91 deletions pkg/engine/lifecycletest/pulumi_test.go
Expand Up @@ -3905,97 +3905,6 @@ func TestAdditionalSecretOutputs(t *testing.T) {
assert.True(t, resA.Outputs["c"].IsSecret())
}

// This test checks that warnings are emitted when options which have no
// effect on components are attached to a component resource.
func TestComponentOptionWarnings(t *testing.T) {
t.Parallel()
type testCase struct {
optionName string
option deploytest.ResourceOptions
}

// These are the options which have no effect on components.
var cases = []testCase{
{
optionName: "retainOnDelete",
option: deploytest.ResourceOptions{
RetainOnDelete: true,
},
}, {
optionName: "ignoreChanges",
option: deploytest.ResourceOptions{
IgnoreChanges: []string{"root"},
},
}, {
optionName: "customTimeouts",
option: deploytest.ResourceOptions{
CustomTimeouts: &resource.CustomTimeouts{},
},
}, {
optionName: "replaceOnChanges",
option: deploytest.ResourceOptions{
ReplaceOnChanges: []string{"*"},
},
}, {
optionName: "additionalSecretOutputs",
option: deploytest.ResourceOptions{
AdditionalSecretOutputs: []resource.PropertyKey{"foobar"},
},
},
}
// This function creates a new language runtime registering a single resource:
// a component registered with the provided option.
var createRuntimeWithOption = func(t *testing.T, option deploytest.ResourceOptions) plugin.LanguageRuntime {
return deploytest.NewLanguageRuntime(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
_, _, _, err := monitor.RegisterResource("component", "resA", false, option)
assert.NoError(t, err)
return nil
})
}

// For each of these scenarios, assert that a component resource
// with this option applied produces a warning.
for _, testCase := range cases {
testCase := testCase
t.Run(testCase.optionName, func(t *testing.T) {
t.Parallel()
var runtime = createRuntimeWithOption(t, testCase.option)
var host = deploytest.NewPluginHost(nil, nil, runtime)
var warningText = fmt.Sprintf("The option '%s' has no effect on component resources.", testCase.optionName)
var p = &TestPlan{
Options: UpdateOptions{Host: host},
Steps: []TestStep{{
Op: Update,
ExpectFailure: false,
SkipPreview: true,
Validate: func(
_ workspace.Project,
_ deploy.Target,
_ JournalEntries,
evts []Event,
res result.Result,
) result.Result {
var foundWarning bool
for _, evt := range evts {
if evt.Type == DiagEvent {
e := evt.Payload().(DiagEventPayload)
msg := colors.Never.Colorize(e.Message)
foundWarning = strings.Contains(msg, warningText) && e.Severity == diag.Warning
if foundWarning {
break
}
}
}
assert.True(t, foundWarning)
return res
},
}},
}
p.Run(t, nil)
})
}
}

func TestDefaultParents(t *testing.T) {
t.Parallel()

Expand Down
7 changes: 2 additions & 5 deletions pkg/resource/deploy/source_eval.go
Expand Up @@ -1260,12 +1260,9 @@ func (rm *resmon) RegisterResource(ctx context.Context,
// This function is intended to validate options passed to component resources,
// so urn is expected to refer to a component.
func (rm *resmon) checkComponentOption(urn resource.URN, optName string, check func() bool) {
var msg = fmt.Sprintf("The option '%s' has no effect on component resources.", optName)
if check() {
rm.diagostics.Warningf(diag.Message(
urn,
msg,
))
logging.V(10).Infof("The option '%s' has no automatic effect on component resource '%s', "+
"ensure it is handled correctly in the component code.", optName, urn)
}
}

Expand Down

0 comments on commit f713611

Please sign in to comment.