Handle additionalProperties (#963)

This should fix
#401.

As stated in the linked issue, the type for the stateful policy of a
managed instance group in the compute module should be a mapping of
string and an object that accepts the value for the auto-delete rule
(enum). The possible values for the auto-delete rule are `NEVER` and
`ON_PERMANENT_INSTANCE_DELETION`.

I looked into this briefly and found that the discovery doc for
`compute.v1` has the right mapping but my guess is that when generating
the schema, the generator is dropping the `$ref` on the
`additionalProperties` which turns these into a simple mapping instead
of a map of maps.

Here's a snippet from the discovery doc on the `master` branch:

```json
"StatefulPolicyPreservedState": {
      "description": "Configuration of preserved resources.",
      "id": "StatefulPolicyPreservedState",
      "properties": {
        "disks": {
          "additionalProperties": {
            "$ref": "StatefulPolicyPreservedStateDiskDevice"
          },
          "description": "Disks created on the instances that will be preserved on instance delete, update, etc. This map is keyed with the device names of the disks.",
          "type": "object"
        },
        "externalIPs": {
          "additionalProperties": {
            "$ref": "StatefulPolicyPreservedStateNetworkIp"
          },
          "description": "External network IPs assigned to the instances that will be preserved on instance delete, update, etc. This map is keyed with the network interface name.",
          "type": "object"
        },
        "internalIPs": {
          "additionalProperties": {
            "$ref": "StatefulPolicyPreservedStateNetworkIp"
          },
          "description": "Internal network IPs assigned to the instances that will be preserved on instance delete, update, etc. This map is keyed with the network interface name.",
          "type": "object"
        }
      },
      "type": "object"
    },
```

And here's the Pulumi schema for `v1`:

```json
"google-native:compute/v1:StatefulPolicy": {
            "properties": {
                "preservedState": {
                    "type": "object",
                    "$ref": "#/types/google-native:compute/v1:StatefulPolicyPreservedState"
                }
            },
            "type": "object"
        },
        "google-native:compute/v1:StatefulPolicyPreservedState": {
            "description": "Configuration of preserved resources.",
            "properties": {
                "disks": {
                    "type": "object",
                    "description": "Disks created on the instances that will be preserved on instance delete, update, etc. This map is keyed with the device names of the disks."
                },
                "externalIPs": {
                    "type": "object",
                    "description": "External network IPs assigned to the instances that will be preserved on instance delete, update, etc. This map is keyed with the network interface name."
                },
                "internalIPs": {
                    "type": "object",
                    "description": "Internal network IPs assigned to the instances that will be preserved on instance delete, update, etc. This map is keyed with the network interface name."
                }
            },
            "type": "object"
        },
```

Note how all properties of `StatefulPolicyPreservedState` are missing
the additionalProperties definition. (I guess the code generator is
defaulting properties of `type: object` without any other attributes as
maps?)

This PR handles `additionalProperties` for both cases where a property
is a map with a primitive value or one where the value is a reference to
a type.

---------

Co-authored-by: Matthew Jeffryes <[email protected]>

.gitignore

@@ -30,3 +30,5 @@ sdk/java/gradlew
 sdk/java/gradlew.bat
 
 sdk/python/venv
+
+go.work.sum

provider/pkg/gen/overrides.go

@@ -188,6 +188,29 @@ var resourceNamePropertyOverrides = map[string]string{
 	"run/v1:Service.servicesId":                               "metadata.name",
 }
 
+// unsupportedPropertiesOverrides is a map of properties in types that
+// are not supported and should be included in the Pulumi schema spec.
+var unsupportedPropertiesOverrides = map[string][]string{
+	// API ref: https://cloud.google.com/service-infrastructure/docs/service-management/reference/rest/v1/services.configs#backendrule
+	"BackendRule": {
+		// The `overridesByRequestProtocol` property in `BackendRule`,
+		// while absent in the API reference docs, is present in the
+		// discover doc that we fetch.
+		//
+		// But it doesn't make sense to include
+		// `overridesByRequestProtocol` in the recursion.
+		// For now don't include this property which means
+		// we avoid the recursion.
+		//
+		// Discovery doc: https://servicemanagement.googleapis.com/$discovery/rest?version=v1
+		"overridesByRequestProtocol",
+	},
+	"BackendRuleResponse": {
+		// Similar to the property in `BackendRule`. See notes above.
+		"overridesByRequestProtocol",
+	},
+}
+
 // autonameOverrides is a map of exceptions to the property used for auto-naming.
 // The key is the resource token, and the value is the property to use for auto-naming.
 var autonameOverrides = map[string]string{

provider/pkg/gen/schema.go

@@ -18,6 +18,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"log"
 	"net/url"
 	"os"
 	"path"
@@ -1249,6 +1250,15 @@ func (g *packageGenerator) genProperties(typeName string, typeSchema *discovery.
 		properties:    map[string]resources.CloudAPIProperty{},
 	}
 	for _, name := range codegen.SortedKeys(typeSchema.Properties) {
+		// Consult the unsupported properties map to see if we should
+		// skip this property altogether.
+		if overrides, ok := unsupportedPropertiesOverrides[typeName]; ok {
+			if contains(overrides, name) {
+				log.Printf("Skipping unsupported property %s in type %s", name, typeName)
+				continue
+			}
+		}
+
 		value := typeSchema.Properties[name]
 		sdkName := apiPropNameToSdkName(typeName, name)
 
@@ -1391,6 +1401,54 @@ func (g *packageGenerator) genTypeSpec(typeName, propName string, prop *discover
 			Type: "object",
 			Ref:  referencedTypeName,
 		}, nil
+	case prop.Type == "object" && prop.AdditionalProperties != nil:
+		// The prop is a map with a string key and a simple value
+		// if it doesn't have a ref.
+		if prop.AdditionalProperties.Ref == "" {
+			switch prop.AdditionalProperties.Type {
+			case "any":
+				return &schema.TypeSpec{
+					Type:                 "object",
+					AdditionalProperties: &schema.TypeSpec{Ref: "pulumi.json#/Any"},
+				}, nil
+			case "array":
+				typeSpec, err := g.genTypeSpec(propName, propName, prop.AdditionalProperties.Items, isOutput)
+				if err != nil {
+					return nil, err
+				}
+
+				return &schema.TypeSpec{
+					Type: "object",
+					AdditionalProperties: &schema.TypeSpec{
+						Type:  "array",
+						Items: typeSpec,
+					},
+				}, nil
+			case "nil":
+				return nil, errors.New(fmt.Sprintf("nil is not a valid array element type: %v", prop))
+			case "object":
+				return nil, errors.New(fmt.Sprintf("object is not a valid array element type: %v", prop))
+			default:
+				return &schema.TypeSpec{
+					Type: "object",
+					AdditionalProperties: &schema.TypeSpec{
+						Type: prop.AdditionalProperties.Type,
+					},
+				}, nil
+			}
+		}
+
+		// Otherwise, the value in-turn is a complex type.
+		typePropName := fmt.Sprintf(`%s%s`, typeName, ToUpperCamel(propName))
+		refTypeSpec, err := g.genTypeSpec(typePropName, propName, prop.AdditionalProperties, isOutput)
+		if err != nil {
+			return nil, errors.New(fmt.Sprintf("error generating type spec for $ref in additional properties %v", err))
+		}
+
+		return &schema.TypeSpec{
+			Type:                 "object",
+			AdditionalProperties: refTypeSpec,
+		}, nil
 	case len(prop.Enum) > 0 && !isOutput:
 		return g.genEnumType(typeName, propName, prop)
 	case prop.Type != "":
@@ -1524,7 +1582,7 @@ func isDeprecated(description string) bool {
 
 // isRequired returns true if the property or a parameter indicates that it is required.
 func isRequired(parameter discovery.JsonSchema) bool {
-	if parameter.Required == true {
+	if parameter.Required {
 		return true
 	}
 	return strings.HasPrefix(parameter.Description, "Required.")

provider/pkg/gen/schema_test.go

@@ -32,6 +32,7 @@ import (
 	"github.com/pulumi/pulumi-google-native/provider/pkg/resources"
 
 	"github.com/stretchr/testify/require"
+	"google.golang.org/api/discovery/v1"
 )
 
 var root = filepath.Join("..", "..", "..")
@@ -213,3 +214,41 @@ func loadSchema() (*schema.PackageSpec, error) {
 	}
 	return &pkg, nil
 }
+
+type TypeSpecTestCase struct {
+	name     string
+	schema   *discovery.JsonSchema
+	expected *schema.TypeSpec
+}
+
+func Test_genTypeSpec(t *testing.T) {
+	cases := []TypeSpecTestCase{
+		{
+			name: "map[string, string]",
+			schema: &discovery.JsonSchema{
+				Type: "object",
+				AdditionalProperties: &discovery.JsonSchema{
+					Type: "string",
+				},
+			},
+			expected: &schema.TypeSpec{
+				Type:                 "object",
+				AdditionalProperties: &schema.TypeSpec{Type: "string"},
+			},
+		},
+	}
+	g := packageGenerator{
+		pkg:          nil,
+		metadata:     nil,
+		rest:         nil,
+		mod:          "foo",
+		visitedTypes: codegen.NewStringSet(),
+		docName:      "foo",
+	}
+
+	for _, tt := range cases {
+		out, err := g.genTypeSpec("foo", "bar", tt.schema, true)
+		assert.NoError(t, err)
+		assert.Equal(t, tt.expected, out)
+	}
+}

provider/pkg/gen/utilities.go

@@ -214,6 +214,16 @@ func toCamelInitCase(s string, initCase bool) string {
 	return n
 }
 
+func contains(slice []string, item string) bool {
+	for _, s := range slice {
+		if s == item {
+			return true
+		}
+	}
+
+	return false
+}
+
 var uppercaseAcronyms = []string{
 	"IP",
 }

sdk/dotnet/Aiplatform/V1/Artifact.cs

@@ -59,7 +59,7 @@ public partial class Artifact : global::Pulumi.CustomResource
         /// Properties of the Artifact. Top level metadata keys' heading and trailing spaces will be trimmed. The size of this field should not exceed 200KB.
         /// </summary>
         [Output("metadata")]
-        public Output<ImmutableDictionary<string, string>> Metadata { get; private set; } = null!;
+        public Output<ImmutableDictionary<string, object>> Metadata { get; private set; } = null!;
 
         [Output("metadataStoreId")]
         public Output<string> MetadataStoreId { get; private set; } = null!;
@@ -194,14 +194,14 @@ public InputMap<string> Labels
         public Input<string>? Location { get; set; }
 
         [Input("metadata")]
-        private InputMap<string>? _metadata;
+        private InputMap<object>? _metadata;
 
         /// <summary>
         /// Properties of the Artifact. Top level metadata keys' heading and trailing spaces will be trimmed. The size of this field should not exceed 200KB.
         /// </summary>
-        public InputMap<string> Metadata
+        public InputMap<object> Metadata
         {
-            get => _metadata ?? (_metadata = new InputMap<string>());
+            get => _metadata ?? (_metadata = new InputMap<object>());
             set => _metadata = value;
         }
 

sdk/dotnet/Aiplatform/V1/Context.cs

@@ -58,7 +58,7 @@ public partial class Context : global::Pulumi.CustomResource
         /// Properties of the Context. Top level metadata keys' heading and trailing spaces will be trimmed. The size of this field should not exceed 200KB.
         /// </summary>
         [Output("metadata")]
-        public Output<ImmutableDictionary<string, string>> Metadata { get; private set; } = null!;
+        public Output<ImmutableDictionary<string, object>> Metadata { get; private set; } = null!;
 
         [Output("metadataStoreId")]
         public Output<string> MetadataStoreId { get; private set; } = null!;
@@ -187,14 +187,14 @@ public InputMap<string> Labels
         public Input<string>? Location { get; set; }
 
         [Input("metadata")]
-        private InputMap<string>? _metadata;
+        private InputMap<object>? _metadata;
 
         /// <summary>
         /// Properties of the Context. Top level metadata keys' heading and trailing spaces will be trimmed. The size of this field should not exceed 200KB.
         /// </summary>
-        public InputMap<string> Metadata
+        public InputMap<object> Metadata
         {
-            get => _metadata ?? (_metadata = new InputMap<string>());
+            get => _metadata ?? (_metadata = new InputMap<object>());
             set => _metadata = value;
         }
 

sdk/dotnet/Aiplatform/V1/Endpoint.cs

@@ -104,7 +104,7 @@ public partial class Endpoint : global::Pulumi.CustomResource
         /// A map from a DeployedModel's ID to the percentage of this Endpoint's traffic that should be forwarded to that DeployedModel. If a DeployedModel's ID is not listed in this map, then it receives no traffic. The traffic percentage values must add up to 100, or map must be empty if the Endpoint is to not accept any traffic at a moment.
         /// </summary>
         [Output("trafficSplit")]
-        public Output<ImmutableDictionary<string, string>> TrafficSplit { get; private set; } = null!;
+        public Output<ImmutableDictionary<string, int>> TrafficSplit { get; private set; } = null!;
 
         /// <summary>
         /// Timestamp when this Endpoint was last updated.
@@ -229,14 +229,14 @@ public InputMap<string> Labels
         public Input<string>? Project { get; set; }
 
         [Input("trafficSplit")]
-        private InputMap<string>? _trafficSplit;
+        private InputMap<int>? _trafficSplit;
 
         /// <summary>
         /// A map from a DeployedModel's ID to the percentage of this Endpoint's traffic that should be forwarded to that DeployedModel. If a DeployedModel's ID is not listed in this map, then it receives no traffic. The traffic percentage values must add up to 100, or map must be empty if the Endpoint is to not accept any traffic at a moment.
         /// </summary>
-        public InputMap<string> TrafficSplit
+        public InputMap<int> TrafficSplit
         {
-            get => _trafficSplit ?? (_trafficSplit = new InputMap<string>());
+            get => _trafficSplit ?? (_trafficSplit = new InputMap<int>());
             set => _trafficSplit = value;
         }