Skip to content

Commit

Permalink
Autonaming configuration in Configure and Check (#2675)
Browse files Browse the repository at this point in the history
This PR implements the bridge part of [Autonaming
Configuration](pulumi/pulumi#1518). See
[RFC](pulumi/pulumi#17592) for the full
design.

In short, [Protobuf definition for autonaming
configuration](pulumi/pulumi#17810) introduced
the protobuf changes required for the provider-side implementation. With
those, a provider can:

- Declare that it supports autonaming configurations with a response
flag in Configure
- Accept two extra properties in CheckRequest: a proposed name and a
mode to apply it

This PR implements autonaming configuration for all bridged providers.
It passes the configuration from CheckRequest all the way down to the
autonaming module. The module itself is now able to understand the modes
and use the proposed name appropriately:

- For `disable` mode, it will disable autonaming and throw an error if
no explicit name was provided by a user
- For `enforce` mode, it will use the proposed name verbatim. Note that
all checks on that name are ignored: this is by design, so that users
would be able to override the provider's settings with `enforce: true`
if they need to
- For `propose` mode, it will try using the proposed name but can also
modify and validate it. The bridge applies transformations (e.g.
lowercasing the name) and checks max length/character set requirements.
See the test cases for details.

No changes are needed to the provider themselves beyond rolling to the
new version of TF bridge once it's published.

The PR is split into three commits: (1) all the manual changes, (2)
mechanical updates of replay tests, (3) end-to-end tests.

Resolves #2722
  • Loading branch information
mikhailshilkov authored Dec 12, 2024
1 parent e53958b commit 7b3ace6
Show file tree
Hide file tree
Showing 31 changed files with 666 additions and 56 deletions.
1 change: 1 addition & 0 deletions dynamic/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ func TestConfigure(t *testing.T) {
}),
}, noParallel, expect(autogold.Expect(`{
"acceptResources": true,
"supportsAutonamingConfiguration": true,
"supportsPreview": true
}`)))(t)

Expand Down
1 change: 1 addition & 0 deletions pkg/pf/internal/defaults/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ func getDefaultValue(
URN: cdOptions.URN,
Properties: cdOptions.Properties,
Seed: cdOptions.Seed,
Autonaming: cdOptions.Autonaming,
})
if err != nil {
msg := fmt.Errorf("Failed computing a default value for property '%s': %w",
Expand Down
27 changes: 27 additions & 0 deletions pkg/pf/internal/defaults/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge"
"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info"
shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim"
"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/schema"
)
Expand Down Expand Up @@ -66,6 +67,10 @@ func TestApplyDefaultInfoValues(t *testing.T) {
return resource.NewStringProperty(unique), err
}

testFromAutoname := func(res *tfbridge.PulumiResource) (interface{}, error) {
return resource.NewStringProperty(res.Autonaming.ProposedName), nil
}

testComputeDefaults := func(
t *testing.T,
expectPriorValue resource.PropertyValue,
Expand Down Expand Up @@ -238,6 +243,28 @@ func TestApplyDefaultInfoValues(t *testing.T) {
"stringProp": resource.NewStringProperty("n1-453"),
},
},
{
name: "From function can compute defaults with autoname",
fieldInfos: map[string]*tfbridge.SchemaInfo{
"string_prop": {
Default: &tfbridge.DefaultInfo{
From: testFromAutoname,
},
},
},
computeDefaultOptions: tfbridge.ComputeDefaultOptions{
URN: "urn:pulumi:test::test::pkgA:index:t1::n1",
Properties: resource.PropertyMap{},
Seed: []byte(`123`),
Autonaming: &info.ComputeDefaultAutonamingOptions{
ProposedName: "n1-777",
Mode: info.ComputeDefaultAutonamingModePropose,
},
},
expected: resource.PropertyMap{
"stringProp": resource.NewStringProperty("n1-777"),
},
},
{
name: "ComputeDefaults function can compute nested defaults",
fieldInfos: map[string]*tfbridge.SchemaInfo{
Expand Down
14 changes: 12 additions & 2 deletions pkg/pf/internal/plugin/provider_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"github.com/pulumi/pulumi/sdk/v3/go/common/workspace"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info"
)

// A version of Provider interface that is enhanced by giving access to the request Context.
Expand All @@ -44,7 +46,8 @@ type ProviderWithContext interface {
ConfigureWithContext(ctx context.Context, inputs resource.PropertyMap) error

CheckWithContext(ctx context.Context, urn resource.URN, olds, news resource.PropertyMap,
allowUnknowns bool, randomSeed []byte) (resource.PropertyMap, []p.CheckFailure, error)
allowUnknowns bool, randomSeed []byte, autonaming *info.ComputeDefaultAutonamingOptions,
) (resource.PropertyMap, []p.CheckFailure, error)

DiffWithContext(ctx context.Context, urn resource.URN, id resource.ID, olds resource.PropertyMap,
news resource.PropertyMap, allowUnknowns bool, ignoreChanges []string) (p.DiffResult, error)
Expand Down Expand Up @@ -148,8 +151,15 @@ func (prov *provider) Configure(
func (prov *provider) Check(
ctx context.Context, req plugin.CheckRequest,
) (plugin.CheckResponse, error) {
var autonaming *info.ComputeDefaultAutonamingOptions
if req.Autonaming != nil {
autonaming = &info.ComputeDefaultAutonamingOptions{
ProposedName: req.Autonaming.ProposedName,
Mode: info.ComputeDefaultAutonamingOptionsMode(req.Autonaming.Mode),
}
}
c, f, err := prov.ProviderWithContext.CheckWithContext(
ctx, req.URN, req.Olds, req.News, req.AllowUnknowns, req.RandomSeed)
ctx, req.URN, req.Olds, req.News, req.AllowUnknowns, req.RandomSeed, autonaming)
return plugin.CheckResponse{Properties: c, Failures: f}, err
}

Expand Down
16 changes: 15 additions & 1 deletion pkg/pf/internal/plugin/provider_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import (
pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge/info"
)

type providerServer struct {
Expand Down Expand Up @@ -333,6 +335,10 @@ func (p *providerServer) Configure(ctx context.Context,
// reason about data flow within the underlying provider (TF), we allow
// the engine to apply its own heuristics.
AcceptSecrets: false,

// Check will accept a configuration property for engine to propose auto-naming format and mode
// when user opts in to control it.
SupportsAutonamingConfiguration: true,
}, nil
}

Expand All @@ -349,7 +355,15 @@ func (p *providerServer) Check(ctx context.Context, req *pulumirpc.CheckRequest)
return nil, err
}

newInputs, failures, err := p.provider.CheckWithContext(ctx, urn, state, inputs, true, req.RandomSeed)
var autonaming *info.ComputeDefaultAutonamingOptions
if req.Autonaming != nil {
autonaming = &info.ComputeDefaultAutonamingOptions{
ProposedName: req.Autonaming.ProposedName,
Mode: info.ComputeDefaultAutonamingOptionsMode(req.Autonaming.Mode),
}
}

newInputs, failures, err := p.provider.CheckWithContext(ctx, urn, state, inputs, true, req.RandomSeed, autonaming)
if err != nil {
return nil, err
}
Expand Down
57 changes: 57 additions & 0 deletions pkg/pf/tests/autonaming_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package tfbridgetests

import (
"testing"

rschema "github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/pulumi/providertest/pulumitest/opttest"
"github.com/stretchr/testify/require"

"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/pf/tests/internal/providerbuilder"
"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/pf/tests/pulcheck"
"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge"
)

func TestAutonaming(t *testing.T) {
t.Parallel()
provBuilder := providerbuilder.NewProvider(
providerbuilder.NewProviderArgs{
AllResources: []providerbuilder.Resource{
providerbuilder.NewResource(providerbuilder.NewResourceArgs{
ResourceSchema: rschema.Schema{
Attributes: map[string]rschema.Attribute{
"name": rschema.StringAttribute{Optional: true},
},
},
}),
},
})

prov := bridgedProvider(provBuilder)
prov.Resources["testprovider_test"] = &tfbridge.ResourceInfo{
Tok: "testprovider:index:Test",
Fields: map[string]*tfbridge.SchemaInfo{
"name": tfbridge.AutoName("name", 50, "-"),
},
}
program := `
name: test
runtime: yaml
config:
pulumi:autonaming:
value:
pattern: ${project}-${name}
resources:
hello:
type: testprovider:index:Test
outputs:
testOut: ${hello.name}
`
opts := []opttest.Option{
opttest.Env("PULUMI_EXPERIMENTAL", "true"),
}
pt, err := pulcheck.PulCheck(t, prov, program, opts...)
require.NoError(t, err)
res := pt.Up(t)
require.Equal(t, "test-hello", res.Outputs["testOut"].Value)
}
6 changes: 4 additions & 2 deletions pkg/pf/tests/provider_check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ func TestCheck(t *testing.T) {
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
}
},
{
Expand Down Expand Up @@ -238,7 +239,8 @@ func TestCheck(t *testing.T) {
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
}
},
{
Expand Down
15 changes: 10 additions & 5 deletions pkg/pf/tests/provider_configure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,8 @@ func TestConfigureToCreate(t *testing.T) {
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
}
},
{
Expand Down Expand Up @@ -527,7 +528,8 @@ func TestConfigureBooleans(t *testing.T) {
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
}
}`)
}
Expand Down Expand Up @@ -610,7 +612,8 @@ func TestJSONNestedConfigure(t *testing.T) {
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
}
}`)
}
Expand All @@ -635,7 +638,8 @@ func TestJSONNestedConfigureWithSecrets(t *testing.T) {
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
}
},
{
Expand Down Expand Up @@ -687,7 +691,8 @@ func TestConfigureWithSecrets(t *testing.T) {
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
}
},
{
Expand Down
6 changes: 4 additions & 2 deletions pkg/pf/tests/provider_read_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ func TestReadFromRefresh(t *testing.T) {
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
},
"metadata": {
"kind": "resource",
Expand Down Expand Up @@ -433,7 +434,8 @@ func TestRefreshSupportsCustomID(t *testing.T) {
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
},
"metadata": {
"kind": "resource",
Expand Down
3 changes: 2 additions & 1 deletion pkg/pf/tests/testdata/genrandom/random-delete-preview.json
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,8 @@
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
},
"metadata": {
"kind": "resource",
Expand Down
3 changes: 2 additions & 1 deletion pkg/pf/tests/testdata/genrandom/random-delete-update.json
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,8 @@
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
},
"metadata": {
"kind": "resource",
Expand Down
3 changes: 2 additions & 1 deletion pkg/pf/tests/testdata/genrandom/random-empty-preview.json
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,8 @@
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
},
"metadata": {
"kind": "resource",
Expand Down
3 changes: 2 additions & 1 deletion pkg/pf/tests/testdata/genrandom/random-empty-update.json
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,8 @@
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
},
"metadata": {
"kind": "resource",
Expand Down
3 changes: 2 additions & 1 deletion pkg/pf/tests/testdata/genrandom/random-initial-preview.json
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
},
"metadata": {
"kind": "resource",
Expand Down
3 changes: 2 additions & 1 deletion pkg/pf/tests/testdata/genrandom/random-initial-update.json
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
},
"metadata": {
"kind": "resource",
Expand Down
3 changes: 2 additions & 1 deletion pkg/pf/tests/testdata/genrandom/random-replace-preview.json
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,8 @@
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
},
"metadata": {
"kind": "resource",
Expand Down
3 changes: 2 additions & 1 deletion pkg/pf/tests/testdata/genrandom/random-replace-update.json
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,8 @@
},
"response": {
"supportsPreview": true,
"acceptResources": true
"acceptResources": true,
"supportsAutonamingConfiguration": true
},
"metadata": {
"kind": "resource",
Expand Down
Loading

0 comments on commit 7b3ace6

Please sign in to comment.