Computed property incorrectly marked Required

[t0yv0]

What happened?

When doing a routine pulumi-oci update pulumi/pulumi-oci#164 schema tools noticed that a global_logging_level property is required. Simplified schema:

func JmsFleetAdvancedFeatureConfigurationDataSource() *schema.Resource {
	return &schema.Resource{
		Schema: map[string]*schema.Schema{
			"lcm": {
				Type:     schema.TypeList,
				Computed: true,
				Elem: &schema.Resource{
					Schema: map[string]*schema.Schema{
						"post_installation_actions": {
							Type:     schema.TypeList,
							Computed: true,
							Elem: &schema.Resource{
								Schema: map[string]*schema.Schema{
									"global_logging_level": {
										Type:     schema.TypeString,
										Computed: true,
									},
								},
							},
						},
					},
				},
			},
		},
	}
}

And

jq '.types["oci:Jms/getFleetAdvancedFeatureConfigurationLcmPostInstallationAction:getFleetAdvancedFeatureConfigurationLcmPostInstallationAction"]' $(find . -name schema.json)   ~/code/pulumi-oci
{
  "properties": {
    "addLoggingHandler": {
      "type": "boolean",
      "description": "Sets FileHandler and ConsoleHandler as handlers in logging.properties file.\n"
    },
    "disabledTlsVersions": {
      "type": "array",
      "items": {
        "type": "string"
      },
      "description": "The following post JRE installation actions are supported by the field:\n* Disable TLS 1.0 , TLS 1.1\n"
    },
    "globalLoggingLevel": {
      "type": "string",
      "description": "Sets the logging level in logging.properties file.\n"
    },
    "minimumKeySizeSettings": {
      "type": "array",
      "items": {
        "$ref": "#/types/oci:Jms/getFleetAdvancedFeatureConfigurationLcmPostInstallationActionMinimumKeySizeSetting:getFleetAdvancedFeatureConfigurationLcmPostInstallationActionMinimumKeySizeSetting"
      },
      "description": "test\n"
    },
    "proxies": {
      "type": "array",
      "items": {
        "$ref": "#/types/oci:Jms/getFleetAdvancedFeatureConfigurationLcmPostInstallationActionProxy:getFleetAdvancedFeatureConfigurationLcmPostInstallationActionProxy"
      },
      "description": "List of proxy properties to be configured in net.properties file.\n"
    },
    "shouldReplaceCertificatesOperatingSystem": {
      "type": "boolean",
      "description": "Restores JDK root certificates with the certificates that are available in the operating system. The following action is supported by the field:\n* Replace JDK root certificates with a list provided by the operating system.\n"
    }
  },
  "type": "object",
  "required": [
    "addLoggingHandler",
    "disabledTlsVersions",
    "globalLoggingLevel",
    "minimumKeySizeSettings",
    "proxies",
    "shouldReplaceCertificatesOperatingSystem"
  ],
  "language": {
    "nodejs": {
      "requiredInputs": []
    }
  }
}

This property is Computed in TF which means that only provider sets it. In Pulumi these are known as Output properties. Currently the schema correctly does not generate an inputProperties entry for it but only a properties (output properties) entry. However the schema lists it in the "required" list, tripping up schema-tools to emit a warning.

We definitely want to avoid the warning but besides simply fixing schema-tools wanted to make sure we fix the root cause and possible mis-understanding here.

In TF properties can be Computed or they can be Computed+Optional. Being Computed only does not make them required, in a sense that provider can still emit null values for them. So languages that track optionality should emit optional/nullable types for these. We need to track this down and perhaps the fix is to not put Computed properties in "required" list in schema ever, whether they are Optional or not.

Expected Behavior

No warning when Computed properties are added.

Steps to reproduce

See above.

Output of pulumi about

N/A

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

[iwahbe]

It looks like output properties can only be required if they are carried over from required input properties. All computed or optional&computed should be optional. It looks like this is what we are trying to do:

pulumi-terraform-bridge/pkg/tfgen/generate.go

Line 578 in f7114fc

return (v.schema != nil && v.schema.Optional() || v.schema.Computed()) || customDefault

I'll see what's going on.

[thomas11]

@iwahbe not sure if pulumi/pulumi-gcp#1125 is a consequence of this issue as well. The required properties in that issue are not computed, though.

[VenelinMartinov]

pulumi-terraform-bridge/pkg/tfgen/generate.go

Line 577 in 9081d29

// If we're checking a property used in an output position, it isn't optional if it's computed.

seems to suggest that this is actually the intended behaviour?

I believe

pulumi-terraform-bridge/pkg/tfgen/generate.go

Line 582 in 224dff6

if v.schema != nil && v.schema.Computed() && !customDefault {

is for outputs if I understand correctly.

I edited it but got quite a big diff in pulumi-oci so not sure if it is correct. Details here #1528

Is there a way to get the tf schema and see details about these properties?

[VenelinMartinov]

After discussing with @t0yv0 it looks like we can not make this change as it would be very disruptive.

For any provider authors who know about specific computed properties which can be null, then they should use MarkAsOptional in the resource SchemaInfo in the provider config. This allows one to override the behaviour.

[pulumi-bot]

Cannot close issue:

• does not have required labels: resolution/

Please fix these problems and try again.