Skip to content

Commit

Permalink
Support invokes with non-object outputs, via wrapping (#3165)
Browse files Browse the repository at this point in the history
Resolves #3147

If we're generating response properties and the resolved schema has no
properties but is a primitive/array output, we include it as a special
property so invokes can be generated.

Ideally we'd just represent the response as a primitive type without the
wrapping magic property, using `FunctionSpec.ReturnType`, but
pulumi/pulumi#15738 and pulumi/pulumi#15739 prevent this.
    
Unfortunately, this change required adding a third boolean to
`genProperties` to limit this new case to function responses only. Happy
to incorporate suggestions to model this better.

This is a simple program that demonstrates one of the new invokes.
```ts
import * as app from "@pulumi/azure-native/app";
const id = app.getCustomDomainVerificationIdOutput();
export const customDomainVerificationId = id;
export const customDomainVerificationIdValue = id.value;
```

Output:
```
  + customDomainVerificationId     : {
      + value: "B11..."
    }
  + customDomainVerificationIdValue: "B11..."
```
  • Loading branch information
thomas11 committed Apr 10, 2024
1 parent 065e8dc commit 3ce1712
Show file tree
Hide file tree
Showing 153 changed files with 7,419 additions and 49 deletions.
136 changes: 134 additions & 2 deletions provider/cmd/pulumi-resource-azure-native/schema.json

Large diffs are not rendered by default.

13 changes: 6 additions & 7 deletions provider/pkg/azure/client.go
Expand Up @@ -31,10 +31,10 @@ type AzureDeleter interface {
type AzureClient interface {
AzureDeleter
CanCreate(ctx context.Context, id, path, apiVersion, readMethod string, isSingletonResource, hasDefaultBody bool, isDefaultResponse func(map[string]any) bool) error
Get(ctx context.Context, id string, apiVersion string) (map[string]interface{}, error)
Get(ctx context.Context, id string, apiVersion string) (any, error)
Head(ctx context.Context, id string, apiVersion string) error
Patch(ctx context.Context, id string, bodyProps map[string]interface{}, queryParameters map[string]interface{}, asyncStyle string) (map[string]interface{}, bool, error)
Post(ctx context.Context, id string, bodyProps map[string]interface{}, queryParameters map[string]interface{}) (map[string]interface{}, error)
Post(ctx context.Context, id string, bodyProps map[string]interface{}, queryParameters map[string]interface{}) (any, error)
Put(ctx context.Context, id string, bodyProps map[string]interface{}, queryParameters map[string]interface{}, asyncStyle string) (map[string]interface{}, bool, error)
}

Expand Down Expand Up @@ -124,7 +124,7 @@ func (a *azureClientImpl) Post(
ctx context.Context,
id string,
bodyProps map[string]interface{},
queryParameters map[string]interface{}) (map[string]interface{}, error) {
queryParameters map[string]interface{}) (any, error) {

preparer := autorest.CreatePreparer(
autorest.AsContentType("application/json; charset=utf-8"),
Expand All @@ -146,7 +146,7 @@ func (a *azureClientImpl) Post(
if err != nil {
return nil, err
}
var outputs map[string]interface{}
var outputs any
err = autorest.Respond(
resp,
a.client.ByInspecting(),
Expand All @@ -159,8 +159,7 @@ func (a *azureClientImpl) Post(
return outputs, nil
}

func (a *azureClientImpl) Get(ctx context.Context, id string,
apiVersion string) (map[string]interface{}, error) {
func (a *azureClientImpl) Get(ctx context.Context, id string, apiVersion string) (any, error) {
queryParameters := map[string]interface{}{
"api-version": apiVersion,
}
Expand All @@ -181,7 +180,7 @@ func (a *azureClientImpl) Get(ctx context.Context, id string,
if err != nil {
return nil, err
}
var outputs map[string]interface{}
var outputs any
err = autorest.Respond(
resp,
a.client.ByInspecting(),
Expand Down
717 changes: 711 additions & 6 deletions provider/pkg/gen/__snapshots__/gen_vnet_test.snap

Large diffs are not rendered by default.

64 changes: 50 additions & 14 deletions provider/pkg/gen/properties.go
Expand Up @@ -43,7 +43,27 @@ type propertyBag struct {

type RequiredContainers [][]string

func (m *moduleGenerator) genProperties(resolvedSchema *openapi.Schema, isOutput, isType bool) (*propertyBag, error) {
// genPropertiesVariant is a set of flags that control the behavior of property generation
// in genProperties and genProperty
type genPropertiesVariant struct {
// isOutput indicates that the properties are being generated for an output type.
isOutput bool
// isType indicates that the properties are being generated for a type definition.
isType bool
// isResponse indicates that the properties are being generated for a response type.
isResponse bool
}

// noResponse returns a new copy of the variant with isResponse set to false.
func (v *genPropertiesVariant) noResponse() genPropertiesVariant {
return genPropertiesVariant{
isOutput: v.isOutput,
isType: v.isType,
isResponse: false,
}
}

func (m *moduleGenerator) genProperties(resolvedSchema *openapi.Schema, variants genPropertiesVariant) (*propertyBag, error) {
result := newPropertyBag()

// Sort properties to make codegen deterministic.
Expand All @@ -60,7 +80,7 @@ func (m *moduleGenerator) genProperties(resolvedSchema *openapi.Schema, isOutput
return nil, err
}

if name == "etag" && !isType && !isOutput {
if name == "etag" && !variants.isType && !variants.isOutput {
// ETags may be defined as inputs to PUT endpoints, but we should never model them as
// resource inputs because they are a protocol implementation detail rather than
// a meaningful desired-state property.
Expand Down Expand Up @@ -98,7 +118,7 @@ func (m *moduleGenerator) genProperties(resolvedSchema *openapi.Schema, isOutput
}

if (ok && flatten && !isDict) || workaroundDelegatedNetworkBreakingChange {
bag, err := m.genProperties(resolvedProperty, isOutput, isType)
bag, err := m.genProperties(resolvedProperty, variants.noResponse())
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -136,14 +156,14 @@ func (m *moduleGenerator) genProperties(resolvedSchema *openapi.Schema, isOutput
}

// Skip read-only properties for input types and write-only properties for output types.
if resolvedProperty.ReadOnly && !isOutput {
if resolvedProperty.ReadOnly && !variants.isOutput {
continue
}
if isOutput && isWriteOnly(resolvedProperty) {
if variants.isOutput && isWriteOnly(resolvedProperty) {
continue
}

propertySpec, apiProperty, err := m.genProperty(name, &property, resolvedSchema.ReferenceContext, resolvedProperty, isOutput, isType)
propertySpec, apiProperty, err := m.genProperty(name, &property, resolvedSchema.ReferenceContext, resolvedProperty, variants)
if err != nil {
return nil, err
}
Expand All @@ -153,7 +173,7 @@ func (m *moduleGenerator) genProperties(resolvedSchema *openapi.Schema, isOutput
continue
}

if isOutput && resolvedProperty.ReadOnly {
if variants.isOutput && resolvedProperty.ReadOnly {
result.requiredSpecs.Add(sdkName)
}

Expand All @@ -171,7 +191,7 @@ func (m *moduleGenerator) genProperties(resolvedSchema *openapi.Schema, isOutput
return nil, err
}

allOfProperties, err := m.genProperties(allOfSchema, isOutput, isType)
allOfProperties, err := m.genProperties(allOfSchema, variants.noResponse())
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -216,18 +236,34 @@ func (m *moduleGenerator) genProperties(resolvedSchema *openapi.Schema, isOutput
result.requiredProperties.Add(name)
}
}

// If the schema has no properties but is a primitive output, we include it as a property so invokes can be generated.
// Ideally we'd just represent it as a primitive type and use FunctionSpec.ReturnType to specify this, but pulumi/pulumi
// #15739 and #15738 prevent this.
if len(names) == 0 && len(result.specs) == 0 && variants.isResponse && len(resolvedSchema.Type) == 1 && resolvedSchema.Type[0] != "object" {
result.specs[resources.SingleValueProperty] = pschema.PropertySpec{
TypeSpec: pschema.TypeSpec{
Type: resolvedSchema.Type[0],
},
Description: result.description,
}
result.properties[resources.SingleValueProperty] = resources.AzureAPIProperty{
Type: resolvedSchema.Type[0],
}
}

return result, nil
}

func (m *moduleGenerator) genProperty(name string, schema *spec.Schema, context *openapi.ReferenceContext, resolvedProperty *openapi.Schema, isOutput, isType bool) (*pschema.PropertySpec, *resources.AzureAPIProperty, error) {
func (m *moduleGenerator) genProperty(name string, schema *spec.Schema, context *openapi.ReferenceContext, resolvedProperty *openapi.Schema, variants genPropertiesVariant) (*pschema.PropertySpec, *resources.AzureAPIProperty, error) {
// Identify properties which are aso available as standalone resources and mark them to be maintained if not specified inline.
// Ignore types as we only support top-level resource properties
// Ignore outputs as this is only affecting the input args of a resource, not the resource outputs.
// We only consider arrays (with Items) because in order for the sub-resource to be a standalone resource it must be able to have many instances.
// There is other kinds of sub-resources where there's only a single instance within the parent resource but these are not handled here.
// They are currently handled by the openapi.default module - where we have to add a special case for them *because* they're not managed by the parent and don't have their own delete method.
maintainSubResourceIfUnset := false
if !isType && !isOutput && schema.Items != nil && schema.Items.Schema != nil {
if !variants.isType && !variants.isOutput && schema.Items != nil && schema.Items.Schema != nil {
itemsRef := schema.Items.Schema.Ref.String()
for _, nestedRef := range m.nestedResourceBodyRefs {
if itemsRef == nestedRef {
Expand All @@ -247,7 +283,7 @@ func (m *moduleGenerator) genProperty(name string, schema *spec.Schema, context
return nil, nil, err
}

typeSpec, err := m.genTypeSpec(name, schema, context, isOutput)
typeSpec, err := m.genTypeSpec(name, schema, context, variants.isOutput)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -300,8 +336,8 @@ func (m *moduleGenerator) genProperty(name string, schema *spec.Schema, context
isStringSet := typeSpec.Type == "object" && typeSpec.AdditionalProperties == nil && typeSpec.Ref == ""

forceNewSpec := noForceNew
if !isOutput {
forceNewSpec = m.forceNew(resolvedProperty, name, isType)
if !variants.isOutput {
forceNewSpec = m.forceNew(resolvedProperty, name, variants.isType)
}

schemaProperty := pschema.PropertySpec{
Expand Down Expand Up @@ -336,7 +372,7 @@ func (m *moduleGenerator) genProperty(name string, schema *spec.Schema, context
}

// Input types only get extra information attached
if !isOutput {
if !variants.isOutput {
if m.isEnum(&schemaProperty.TypeSpec) {
metadataProperty = resources.AzureAPIProperty{
Type: "string",
Expand Down
94 changes: 94 additions & 0 deletions provider/pkg/gen/properties_test.go
Expand Up @@ -5,7 +5,9 @@ import (

"github.com/go-openapi/spec"
"github.com/pulumi/pulumi-azure-native/v2/provider/pkg/openapi"
"github.com/pulumi/pulumi-azure-native/v2/provider/pkg/resources"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func makeSchema(mutability ...string) *openapi.Schema {
Expand Down Expand Up @@ -100,3 +102,95 @@ func TestForceNew(t *testing.T) {
assert.Equal(t, noForceNew, forceNewMetadata)
})
}

func TestNonObjectInvokeResponses(t *testing.T) {
m := moduleGenerator{
prov: "foo",
}

resolvedSchema := &openapi.Schema{
Schema: &spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"string"},
},
},
}

t.Run("string type, response properties requested", func(t *testing.T) {
variant := genPropertiesVariant{
isOutput: true,
isType: false,
isResponse: true,
}
props, err := m.genProperties(resolvedSchema, variant)
require.NoError(t, err)

require.Len(t, props.specs, 1)
assert.Contains(t, props.specs, resources.SingleValueProperty)

require.Len(t, props.properties, 1)
assert.Contains(t, props.properties, resources.SingleValueProperty)
})

t.Run("string type, response properties not requested", func(t *testing.T) {
variant := genPropertiesVariant{
isOutput: true,
isType: false,
isResponse: false,
}
props, err := m.genProperties(resolvedSchema, variant)
require.NoError(t, err)

require.Len(t, props.specs, 0)
require.Len(t, props.properties, 0)
})

t.Run("object type, response properties requested", func(t *testing.T) {
schema := &openapi.Schema{
Schema: &spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"object"},
},
},
}

variant := genPropertiesVariant{
isOutput: true,
isType: false,
isResponse: true,
}
props, err := m.genProperties(schema, variant)
require.NoError(t, err)

require.Len(t, props.specs, 0)
require.Len(t, props.properties, 0)
})

t.Run("string type, response properties requested but there are other properties", func(t *testing.T) {
schema := &openapi.Schema{
Schema: &spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"string"},
Properties: map[string]spec.Schema{
"foo": {
SchemaProps: spec.SchemaProps{},
},
},
},
},
}

variant := genPropertiesVariant{
isOutput: true,
isType: false,
isResponse: true,
}
props, err := m.genProperties(schema, variant)
require.NoError(t, err)

require.Len(t, props.specs, 1)
assert.NotContains(t, props.specs, resources.SingleValueProperty)
require.Len(t, props.properties, 1)
assert.NotContains(t, props.properties, resources.SingleValueProperty)
})
}
12 changes: 10 additions & 2 deletions provider/pkg/gen/schema.go
Expand Up @@ -1311,7 +1311,11 @@ func (m *moduleGenerator) genMethodParameters(parameters []spec.Parameter, ctx *
}

// The body parameter is flattened, so that all its properties become the properties of the type.
props, err := m.genProperties(bodySchema, false /* isOutput */, false /* isType */)
props, err := m.genProperties(bodySchema, genPropertiesVariant{
isOutput: false,
isType: false,
isResponse: false,
})
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1394,7 +1398,11 @@ func (m *moduleGenerator) genResponse(statusCodeResponses map[int]spec.Response,
return nil, nil
}

result, err := m.genProperties(responseSchema, true /* isOutput */, false /* isType */)
result, err := m.genProperties(responseSchema, genPropertiesVariant{
isOutput: true,
isType: false,
isResponse: true,
})
if err != nil {
return nil, err
}
Expand Down
11 changes: 9 additions & 2 deletions provider/pkg/gen/types.go
Expand Up @@ -78,7 +78,11 @@ func (m *moduleGenerator) genTypeSpec(propertyName string, schema *spec.Schema,
if _, ok := m.visitedTypes[tok]; !ok {
m.visitedTypes[tok] = true

props, err := m.genProperties(resolvedSchema, isOutput, true /* isType */)
props, err := m.genProperties(resolvedSchema, genPropertiesVariant{
isOutput: isOutput,
isType: true,
isResponse: false,
})
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -176,7 +180,10 @@ Example of a relative ID: $self/frontEndConfigurations/my-frontend.`
if err != nil {
return nil, err
}
itemsSpec, _, err := m.genProperty(propertyName, resolvedSchema.Items.Schema, context, resolvedProperty, isOutput, true /* isType */)
itemsSpec, _, err := m.genProperty(propertyName, resolvedSchema.Items.Schema, context, resolvedProperty, genPropertiesVariant{
isOutput: isOutput,
isType: true,
})
if err != nil {
return nil, err
}
Expand Down

0 comments on commit 3ce1712

Please sign in to comment.