Nested readOnlyProperties are included in "writeable" properties

[mrgrain]

This is a bug relating to deeply nested readonly properties making it into the service-spec as writeable properties. The practical bug is that in the AWS CDK, we currently expose some nested props that can in fact not be set by the user. Throughout this issue, I'll be using EffectiveEngineVersion as an example. However the issue exists for other cases as well.

Background

The CFN Resource Provider Schema has a concept of readOnlyProperties. For the AWS::Athena::WorkGroup schema, these are defined as:

  "readOnlyProperties": [
    "/properties/CreationTime",
    "/properties/WorkGroupConfiguration/EngineVersion/EffectiveEngineVersion",
    "/properties/WorkGroupConfigurationUpdates/EngineVersion/EffectiveEngineVersion"
  ],

These can reference properties, but also can reference properties INSIDE type definitions that are used as properties. In an orchestrator like CloudFormation, you would access them like this:

{ "Fn::GetAtt": ["MyAthena", "CreationTime"] }
{ "Fn::GetAtt": ["MyAthena", "WorkGroupConfiguration.EngineVersion.EffectiveEngineVersion"] }
{ "Fn::GetAtt": ["MyAthena", "WorkGroupConfigurationUpdates.EngineVersion.EffectiveEngineVersion"] }

Notice how the /es used to reference a value inside a complex property get turned into a . in this calling convention. The fullstops . are misleading here, as they are hinting at some sort of object access. However from an API point of view, this is really just a name with somewhat special characters.

Additionally: every property in readOnlyProperties is never writable. This might be obvious given the name, but worth calling out.

The readOnlyProperty reference /properties/WorkGroupConfiguration/EngineVersion/EffectiveEngineVersion points to a property inside another type definition. In this case, inside a type definition called EngineVersion:

    "EngineVersion" : {
      "description" : "The Athena engine version for running queries.",
      "type" : "object",
      "properties" : {
        "SelectedEngineVersion" : {
          "$ref" : "#/definitions/SelectedEngineVersion"
        },
        "EffectiveEngineVersion" : {
          "$ref" : "#/definitions/EffectiveEngineVersion"
        }
      },
      "additionalProperties" : false
    },

This EngineVersion type definition is also used as the input type for the WorkGroupConfiguration/EngineVersion property. So it runs double duty: it is the type of an input property, as well as on the path to an output attribute.

The bug

This currently leads the EngineVersion type definition to have 2 configurable fields: selectedEngineVersion and effectiveEngineVersion... even though effectiveEngineVersion is not actually configurable, and trying to specify it is an error.

For the example of EffectiveEngineVersion this is the service-spec type EngineVersion:

        {
          "$id": "564",
          "name": "EngineVersion",
          "documentation": "The Athena engine version for running queries, or the PySpark engine version for running sessions.",
          "properties": {
            "SelectedEngineVersion": {
              "type": { "type": "string" },
              "documentation": "The engine version requested by the user. Possible values are determined by the output of `ListEngineVersions` , including AUTO. The default is AUTO."
            },
            "EffectiveEngineVersion": {
              "type": { "type": "string" },
              "documentation": "Read only. The engine version on which the query runs. If the user requests a valid engine version other than Auto, the effective engine version is the same as the engine version that the user requested. If the user requests Auto, the effective engine version is chosen by Athena. When a request to update the engine version is made by a `CreateWorkGroup` or `UpdateWorkGroup` operation, the `EffectiveEngineVersion` field is ignored."
            }
          }
        },

As you can see, it incorrectly includes EffectiveEngineVersion.

In practices this results in CDK including this non-writable value in the property interface: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_athena.CfnWorkGroup.EngineVersionProperty.html

The fix

Since:

• awscdk-service-spec is targeted at CloudFormation users;
• the type definition [of EngineVersion] is only used to type input properties, not to type output properties;
• deep GetAttable attributes are referenced as string names that happen to have . in them

The correct fix seems to be to remove readOnlyProperties from their type definitions.

However, also compare Lambda's SnapStartResponse -- that is a structure that is retrievable via { Fn::GetAtt }, and its fields are individually retrievable as well... so removing those properties from the type will leave the SnapStartResponse type empty. So perhaps the rule should be to only do this for types that are also used as input types. Or generate 2 different type definitions for Registry Schema definitions that are used for input and output. Or we just don't list attributes that have a complex output value.

Related

(See also #1139 for more work around readOnlyProperties. Confusion around the difference between readOnlyProperties and GetAtt led to the implementation of the incorrect behavior we're seeing today)