Skip to content

[Fix] Allow child resources to override parent's protect setting #2156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iwahbe merged 3 commits into from
May 12, 2026
Merged

[Fix] Allow child resources to override parent's protect setting #2156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4580ff5
[Fix] Allow child resources to override parent's protect setting
addu390 May 2, 2026
3a95add
Make protect, retainOnDelete and deleteBeforeReplace tri-state
addu390 May 3, 2026
23e8bfc
Add changelog entry
addu390 May 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/bug-fixes-2156.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
component: sdk
kind: bug-fixes
body: Fix child resources unable to override protect, retainOnDelete, and deleteBeforeReplace from parent.
time: 2026-05-11T14:57:14.03221-04:00
custom:
PR: "2156"
1 change: 0 additions & 1 deletion pkg/cmd/pulumi-language-java/language_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ var expectedFailures = map[string]string{
"l2-component-component-resource-ref": "components with resources as inputs/outputs not supported",
"l2-component-program-resource-ref": "components with resources as inputs/outputs not supported",
"l2-resource-secret": "#1564 Fix l2-resource-secret",
"l2-resource-parent-inheritance": "Fix l2-resource-parent-inheritance",
"l2-namespaced-provider": "components with resources as inputs/outputs not supported",
"l2-component-property-deps": "compilation error",
"l2-parameterized-resource": "compilation error",
Expand Down
4 changes: 3 additions & 1 deletion ...testdata/projects/l2-resource-parent-inheritance/src/main/java/generated_program/App.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
import com.pulumi.simple.Resource;
import com.pulumi.simple.ResourceArgs;
import com.pulumi.resources.CustomResourceOptions;
import java.util.List;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Map;
import java.io.File;
import java.nio.file.Files;
Expand Down Expand Up @@ -42,6 +42,7 @@ public static void stack(Context ctx) {
.value(true)
.build(), CustomResourceOptions.builder()
.protect(true)
.retainOnDelete(true)
.build());

var child2 = new Resource("child2", ResourceArgs.builder()
Expand All @@ -54,6 +55,7 @@ public static void stack(Context ctx) {
.value(true)
.build(), CustomResourceOptions.builder()
.protect(false)
.retainOnDelete(false)
.parent(parent2)
.build());

Expand Down
14 changes: 9 additions & 5 deletions sdk/java/pulumi/src/main/java/com/pulumi/deployment/internal/DeploymentImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1685,14 +1685,11 @@ private RegisterResourceRequest createRegisterResourceRequest(
.setType(type)
.setName(name)
.setCustom(custom)
.setProtect(options.isProtect())
.setVersion(options.getVersion().orElse(""))
.setPluginDownloadURL(options.getPluginDownloadURL().orElse(""))
.setImportId(customOpts ? ((CustomResourceOptions) options).getImportId().orElse("") : "")
.setAcceptSecrets(true)
.setAcceptResources(!this.disableResourceReferences)
.setDeleteBeforeReplace(customOpts && ((CustomResourceOptions) options).getDeleteBeforeReplace())
.setDeleteBeforeReplaceDefined(true)
.setCustomTimeouts(
RegisterResourceRequest.CustomTimeouts.newBuilder()
.setCreate(customTimeoutToGolangString.apply(CustomTimeouts::getCreate))
Expand All @@ -1701,11 +1698,18 @@ private RegisterResourceRequest createRegisterResourceRequest(
.build()
)
.setRemote(remote)
.setRetainOnDelete(options.isRetainOnDelete())
.setPackageRef(packageRef == null ? "" : packageRef);

options.getProtect().ifPresent(request::setProtect);
options.getRetainOnDelete().ifPresent(request::setRetainOnDelete);

if (customOpts) {
request.addAllAdditionalSecretOutputs(((CustomResourceOptions) options).getAdditionalSecretOutputs());
var customResourceOpts = (CustomResourceOptions) options;
customResourceOpts.getDeleteBeforeReplaceOptional().ifPresent(value -> {
request.setDeleteBeforeReplace(value);
request.setDeleteBeforeReplaceDefined(true);
});
request.addAllAdditionalSecretOutputs(customResourceOpts.getAdditionalSecretOutputs());
request.addAllReplaceOnChanges(options.getReplaceOnChanges());
}

Expand Down
4 changes: 2 additions & 2 deletions sdk/java/pulumi/src/main/java/com/pulumi/resources/ComponentResourceOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ public ComponentResourceOptions(
@Nullable Output<String> id,
@Nullable Resource parent,
@Nullable Output<List<Resource>> dependsOn,
boolean protect,
@Nullable Boolean protect,
@Nullable List<String> ignoreChanges,
@Nullable String version,
@Nullable CustomTimeouts customTimeouts,
@Nullable List<ResourceTransformation> resourceTransformations,
@Nullable List<Output<Alias>> aliases,
@Nullable String urn,
@Nullable List<String> replaceOnChanges,
boolean retainOnDelete,
@Nullable Boolean retainOnDelete,
@Nullable String pluginDownloadURL,
@Nullable List<String> hideDiffs,
@Nullable List<ProviderResource> providers,
Expand Down
28 changes: 22 additions & 6 deletions sdk/java/pulumi/src/main/java/com/pulumi/resources/CustomResourceOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ public final class CustomResourceOptions extends ResourceOptions implements Copy

public static final CustomResourceOptions Empty = CustomResourceOptions.builder().build();

private boolean deleteBeforeReplace;
@Nullable
private Boolean deleteBeforeReplace;
@Nullable
private List<String> additionalSecretOutputs;
@Nullable
Expand All @@ -31,19 +32,19 @@ private CustomResourceOptions(
@Nullable Output<String> id,
@Nullable Resource parent,
@Nullable Output<List<Resource>> dependsOn,
boolean protect,
@Nullable Boolean protect,
@Nullable List<String> ignoreChanges,
@Nullable String version,
@Nullable ProviderResource provider,
@Nullable CustomTimeouts customTimeouts,
@Nullable List<ResourceTransformation> resourceTransformations,
@Nullable List<Output<Alias>> aliases,
@Nullable String urn,
boolean deleteBeforeReplace,
@Nullable Boolean deleteBeforeReplace,
@Nullable List<String> additionalSecretOutputs,
@Nullable String importId,
@Nullable List<String> replaceOnChanges,
boolean retainOnDelete,
@Nullable Boolean retainOnDelete,
@Nullable String pluginDownloadURL,
@Nullable List<String> hideDiffs,
@Nullable List<Resource> replaceWith,
Expand Down Expand Up @@ -75,6 +76,14 @@ public Builder deleteBeforeReplace(boolean deleteBeforeReplace) {
return this;
}

/**
* @see #deleteBeforeReplace(boolean)
*/
public Builder deleteBeforeReplace(@Nullable Boolean deleteBeforeReplace) {
options.deleteBeforeReplace = deleteBeforeReplace;
return this;
}

/**
* The names of outputs for this resource that should be treated as secrets. This augments
* the list that the resource provider and pulumi engine already determine based on inputs
Expand Down Expand Up @@ -114,7 +123,14 @@ public CustomResourceOptions build() {
* @see Builder#deleteBeforeReplace(boolean)
*/
public boolean getDeleteBeforeReplace() {
return this.deleteBeforeReplace;
return this.deleteBeforeReplace != null && this.deleteBeforeReplace;
}

/**
* @see Builder#deleteBeforeReplace(Boolean)
*/
public Optional<Boolean> getDeleteBeforeReplaceOptional() {
return Optional.ofNullable(this.deleteBeforeReplace);
}

/**
Expand Down Expand Up @@ -188,7 +204,7 @@ public static CustomResourceOptions merge(
//noinspection ConstantConditions
options1 = mergeSharedOptions(options1, options2, id);

options1.deleteBeforeReplace = options1.deleteBeforeReplace || options2.deleteBeforeReplace;
options1.deleteBeforeReplace = options2.deleteBeforeReplace == null ? options1.deleteBeforeReplace : options2.deleteBeforeReplace;
options1.importId = options2.importId == null ? options1.importId : options2.importId;

options1.additionalSecretOutputs = mergeNullableList(options1.additionalSecretOutputs, options2.additionalSecretOutputs);
Expand Down
3 changes: 1 addition & 2 deletions sdk/java/pulumi/src/main/java/com/pulumi/resources/Resource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,6 @@ protected Resource(
// the 'childResources' is a Synchronized Collection, so this is safe operation
parentResource.childResources.add(this);

options.protect = options.protect || parentResource.protect; // TODO: is this logic good?
thisProviders.putAll(options.parent.providers);
}

Expand Down Expand Up @@ -237,7 +236,7 @@ protected Resource(
thisProviders.putAll(convertToProvidersMap(providerList));
}

this.protect = options.protect;
this.protect = options.protect != null && options.protect;
this.provider = custom ? options.provider : null;
this.version = options.version;
this.providers = Map.copyOf(thisProviders);
Expand Down
52 changes: 42 additions & 10 deletions sdk/java/pulumi/src/main/java/com/pulumi/resources/ResourceOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ public abstract class ResourceOptions {
protected Resource parent;
@Nullable
protected Output<List<Resource>> dependsOn;
protected boolean protect;
@Nullable
protected Boolean protect;
@Nullable
protected List<String> ignoreChanges;
@Nullable
Expand All @@ -40,7 +41,8 @@ public abstract class ResourceOptions {
protected String urn;
@Nullable
protected List<String> replaceOnChanges;
protected boolean retainOnDelete;
@Nullable
protected Boolean retainOnDelete;
@Nullable
protected String pluginDownloadURL;
@Nullable
Expand All @@ -56,7 +58,7 @@ protected ResourceOptions(
@Nullable Output<String> id,
@Nullable Resource parent,
@Nullable Output<List<Resource>> dependsOn,
boolean protect,
@Nullable Boolean protect,
@Nullable List<String> ignoreChanges,
@Nullable String version,
@Nullable ProviderResource provider,
Expand All @@ -65,7 +67,7 @@ protected ResourceOptions(
@Nullable List<Output<Alias>> aliases,
@Nullable String urn,
@Nullable List<String> replaceOnChanges,
boolean retainOnDelete,
@Nullable Boolean retainOnDelete,
@Nullable String pluginDownloadURL,
@Nullable List<String> hideDiffs,
@Nullable List<Resource> replaceWith,
Expand Down Expand Up @@ -159,6 +161,15 @@ public B protect(boolean protect) {
return (B) this;
}

/**
* @see #protect(boolean)
*/
public B protect(@Nullable Boolean protect) {
options.protect = protect;
//noinspection unchecked
return (B) this;
}

/**
* Ignore changes to any of the specified properties.
*/
Expand Down Expand Up @@ -285,8 +296,17 @@ public B replaceOnChanges(@Nullable List<String> replaceOnChanges) {
* If set to True, the providers Delete method will not be called for this resource.
*/
public B retainOnDelete(boolean retainOnDelete) {
options.retainOnDelete = retainOnDelete;
//noinspection unchecked
return (B) this;
}

/**
* @see #retainOnDelete(boolean)
*/
public B retainOnDelete(@Nullable Boolean retainOnDelete) {
options.retainOnDelete = retainOnDelete;
//noinspection unchecked
return (B) this;
}

Expand Down Expand Up @@ -370,7 +390,14 @@ public Output<List<Resource>> getDependsOn() {
* @see Builder#protect(boolean)
*/
public boolean isProtect() {

@Frassle Frassle May 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what's actually used to set the RegisterResource request, so I think we need another method here "isProtectNullable"(?) to check in the register resource code to work out if we should actually send true, false, or null to the engine.

Also it would be good to get the l2-resource-parent-inheritance test passing with this change, even if it requires a manually written program file.

@addu390 addu390 May 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Frassle Went with Optional<Boolean> getProtect() instead, similar to the other Optional<> getters.

And for the test: retainOnDelete needed the same tri-state fix to make it pass, and while in there, deleteBeforeReplace had the same bug. Saw your draft in #1956 was heading the same way, so applied the pattern across all three.

Tests passed locally.

return protect;
return protect != null && protect;
}

/**
* @see Builder#protect(Boolean)
*/
public Optional<Boolean> getProtect() {
return Optional.ofNullable(protect);
}

/**
Expand Down Expand Up @@ -433,7 +460,14 @@ public List<String> getReplaceOnChanges() {
* @see Builder#retainOnDelete(boolean)
*/
public boolean isRetainOnDelete() {
return this.retainOnDelete;
return this.retainOnDelete != null && this.retainOnDelete;
}

/**
* @see Builder#retainOnDelete(Boolean)
*/
public Optional<Boolean> getRetainOnDelete() {
return Optional.ofNullable(this.retainOnDelete);
}

/**
Expand Down Expand Up @@ -477,7 +511,7 @@ protected static <T extends ResourceOptions> T mergeSharedOptions(T options1, T

options1.id = options2.id == null ? options1.id : options2.id;
options1.parent = options2.parent == null ? options1.parent : options2.parent;
options1.protect = options1.protect || options2.protect;
options1.protect = options2.protect == null ? options1.protect : options2.protect;
options1.urn = options2.urn == null ? options1.urn : options2.urn;
options1.version = options2.version == null ? options1.version : options2.version;
options1.provider = options2.provider == null ? options1.provider : options2.provider;
Expand All @@ -487,9 +521,7 @@ protected static <T extends ResourceOptions> T mergeSharedOptions(T options1, T
options1.resourceTransformations = mergeNullableList(options1.resourceTransformations, options2.resourceTransformations);
options1.aliases = mergeNullableList(options1.aliases, options2.aliases);
options1.replaceOnChanges = mergeNullableList(options1.replaceOnChanges, options2.replaceOnChanges);
options1.retainOnDelete = options1.retainOnDelete || options2.retainOnDelete;
options1.pluginDownloadURL = options2.pluginDownloadURL == null ? options1.pluginDownloadURL : options2.pluginDownloadURL;
options1.retainOnDelete = options1.retainOnDelete || options2.retainOnDelete;
options1.retainOnDelete = options2.retainOnDelete == null ? options1.retainOnDelete : options2.retainOnDelete;
options1.pluginDownloadURL = options2.pluginDownloadURL == null ? options1.pluginDownloadURL : options2.pluginDownloadURL;
options1.hideDiffs = mergeNullableList(options1.hideDiffs, options2.hideDiffs);
options1.dependsOn = Output.concatList(options1.dependsOn, options2.dependsOn);
Expand Down
8 changes: 4 additions & 4 deletions sdk/java/pulumi/src/test/java/com/pulumi/resources/ResourceOptionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ class ResourceOptionsTest {
private static Stream<Arguments> testMergeSharedOptions() {
return Stream.of(
arguments(new TestResourceOptions(), new TestResourceOptions(), new TestResourceOptions(
null, null, Output.of(List.of()), false, null,
null, null, null, null, null, null, null, false, null, null, null, null
null, null, Output.of(List.of()), null, null,
null, null, null, null, null, null, null, null, null, null, null, null
)),
arguments( new TestResourceOptions(
null,
Expand Down Expand Up @@ -115,7 +115,7 @@ public TestResourceOptions(
@Nullable Output<String> id,
@Nullable Resource parent,
@Nullable Output<List<Resource>> dependsOn,
boolean protect,
@Nullable Boolean protect,
@Nullable List<String> ignoreChanges,
@Nullable String version,
@Nullable ProviderResource provider,
Expand All @@ -124,7 +124,7 @@ public TestResourceOptions(
@Nullable List<Output<Alias>> aliases,
@Nullable String urn,
@Nullable List<String> replaceOnChanges,
boolean retainOnDelete,
@Nullable Boolean retainOnDelete,
@Nullable String pluginDownloadURL,
@Nullable List<String> hideDiffs,
@Nullable List<Resource> replaceWith,
Expand Down
Loading