Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose provider config #164

Merged
merged 23 commits into from
Mar 7, 2025
Merged

Expose provider config #164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d32b3be
init
Zaid-Ajaj Feb 28, 2025
b2a9cc3
Merge branch 'main' into zaid/exposing-provider-configuration
Zaid-Ajaj Feb 28, 2025
e6eaeda
initial implementation writing out config to TF files
Zaid-Ajaj Feb 28, 2025
17ed559
Merge branch 'main' into zaid/exposing-provider-configuration
Zaid-Ajaj Feb 28, 2025
c8f8b19
remove in-place test artifacts
Zaid-Ajaj Feb 28, 2025
4d7cac8
lint
Zaid-Ajaj Feb 28, 2025
d68e485
add unit test for cleanProvidersConfig
Zaid-Ajaj Mar 3, 2025
68c7136
Permit detecting timeout
t0yv0 Mar 3, 2025
9bf690a
Fix self-references
t0yv0 Mar 3, 2025
f96880c
Merge pull request #171 from pulumi/t0yv0/exposing-provider-configura…
Zaid-Ajaj Mar 4, 2025
b445d1d
clean up and lint
Zaid-Ajaj Mar 4, 2025
01fdbe9
Emit generated TF files in tests
Zaid-Ajaj Mar 5, 2025
6cdcd6c
Keep unknowns and secrets when configuring explicit providers
Zaid-Ajaj Mar 5, 2025
449e1f3
Merge branch 'main' into zaid/exposing-provider-configuration
Zaid-Ajaj Mar 5, 2025
d914003
fix test
Zaid-Ajaj Mar 5, 2025
73ec203
lint
Zaid-Ajaj Mar 5, 2025
c3a3b1d
Mimic CheckConfig in TestSavingModuleState
Zaid-Ajaj Mar 5, 2025
ffeaf56
Make provider resource available only when URN is known
Zaid-Ajaj Mar 5, 2025
146ae16
Merge branch 'main' into zaid/exposing-provider-configuration
Zaid-Ajaj Mar 5, 2025
b2b7884
fix state params
Zaid-Ajaj Mar 5, 2025
40c76ba
acquire provider config for Delete from Configure(...)
Zaid-Ajaj Mar 6, 2025
5f1aaf5
address comments
Zaid-Ajaj Mar 7, 2025
84740b2
bring back skipLocalRunsWithoutCreds for TestS3BucketModSecret
Zaid-Ajaj Mar 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 53 additions & 4 deletions pkg/modprovider/module_component.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ func NewModuleComponentResource(
moduleInputs resource.PropertyMap,
inferredModule *InferredModuleSchema,
packageRef string,
providerSelfURN pulumi.URN,
providersConfig map[string]resource.PropertyMap,
opts ...pulumi.ResourceOption,
) (componentUrn *urn.URN, outputs pulumi.Input, finalError error) {
component := ModuleComponentResource{}
Expand All @@ -84,15 +86,28 @@ func NewModuleComponentResource(
planStore.Forget(urn)
}()

var providerSelfRef pulumi.ProviderResource
if providerSelfURN != "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remind me why it's ever empty. Is it only empty in tests? If so can we add a comment to that purpose?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is empty if only CheckConfig is called until we can access the URN from Configure. Right now, URN from Configure is not strictly needed and the check here is a workaround for TestSavingModuleState

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK yeah TestSavingModuleState we might want to remove that, together with an incomplete engine, in favor of just trusting full blown integration tests.

providerSelfRef = newProviderSelfReference(ctx, providerSelfURN)
}

go func() {
resourceOptions := []pulumi.ResourceOption{
pulumi.Parent(&component),
}

if providerSelfRef != nil {
resourceOptions = append(resourceOptions, pulumi.Provider(providerSelfRef))
}

_, err := newModuleStateResource(ctx,
// Needs to be prefixed by parent to avoid "duplicate URN".
fmt.Sprintf("%s-state", name),
pkgName,
urn,
packageRef,
moduleInputs,
pulumi.Parent(&component),
resourceOptions...,
)

contract.AssertNoErrorf(err, "newModuleStateResource failed")
Expand Down Expand Up @@ -127,7 +142,10 @@ func NewModuleComponentResource(
Name: outputName,
})
}
err = tfsandbox.CreateTFFile(tfName, tfModuleSource, tfModuleVersion, tf.WorkingDir(), moduleInputs, outputSpecs)
err = tfsandbox.CreateTFFile(tfName, tfModuleSource,
tfModuleVersion, tf.WorkingDir(),
moduleInputs, outputSpecs, providersConfig)

if err != nil {
return nil, nil, fmt.Errorf("Seed file generation failed: %w", err)
}
Expand Down Expand Up @@ -169,10 +187,19 @@ func NewModuleComponentResource(
return
}

resourceOptions := []pulumi.ResourceOption{
pulumi.Parent(&component),
}

if providerSelfRef != nil {
resourceOptions = append(resourceOptions, pulumi.Provider(providerSelfRef))
}

cr, err := newChildResource(ctx, urn, pkgName,
rp,
packageRef,
pulumi.Parent(&component))
resourceOptions...,
)

errs = append(errs, err)
if err == nil {
Expand Down Expand Up @@ -211,10 +238,19 @@ func NewModuleComponentResource(
// so that we propagate outputs from module
return
}

resourceOptions := []pulumi.ResourceOption{
pulumi.Parent(&component),
}

if providerSelfRef != nil {
resourceOptions = append(resourceOptions, pulumi.Provider(providerSelfRef))
}

cr, err := newChildResource(ctx, urn, pkgName,
rp,
packageRef,
pulumi.Parent(&component))
resourceOptions...)

errs = append(errs, err)
if err == nil {
Expand Down Expand Up @@ -242,3 +278,16 @@ func NewModuleComponentResource(

return &urn, marshalledOutputs, nil
}

func newProviderSelfReference(ctx *pulumi.Context, urn1 pulumi.URN) pulumi.ProviderResource {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Frassle if I could have your 5 min to eyeball this.

We feed the result of this function to pulumi.Provider(newProviderSelfReference(..)). This is a workaround. I tried and tired to find a way to pass URN directly to pulumi.Provider() but could not :/

We could drop to gRPC level of calling RegisterResource to workaround this but that negates benefits of using the SDK.

While this workaround seems to work, you were mentioning that self-identifying a provider instance requires both a self-URN and self-ID, and self-URN by itself is not specific enough.

Ideally something like this maybe would be public?
https://github.com/pulumi/pulumi/blob/68295c45f3f3c8f6aadbd76d141a4dcf4f0a55d2/sdk/go/pulumi/resource.go#L181

I found no way to call it though due to internal constraints.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It tried extending the struct and overriding ID() and URN() outputs. That worked EXCEPT I cannot seem to ever be able to set this highly private pkg field, not through reflection, not through subtyping, nor can I override getPackage so I am stuck not being able to make it work with the Provider option.

https://github.com/pulumi/pulumi/blob/68295c45f3f3c8f6aadbd76d141a4dcf4f0a55d2/sdk/go/pulumi/resource.go#L193

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could use what we use here, the code as is, and then wrap{prov} with

type wrap struct {
   pulumi.ProviderResourceState
   id pulumi.ID
}

func (*wrap) ID() pulumi.IDOutput{
   return toOutput(pulumi.ID)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#191

var prov pulumi.ProviderResourceState
err := ctx.RegisterResource(
string(urn.URN(urn1).Type()),
urn.URN(urn1).Name(),
pulumi.Map{},
&prov,
pulumi.URN_(string(urn1)),
)
contract.AssertNoErrorf(err, "RegisterResource failed to hydrate a self-reference")
return &prov
}
3 changes: 3 additions & 0 deletions pkg/modprovider/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ func pulumiSchemaForModule(pargs *ParameterizeArgs, inferredModule *InferredModu
Name: string(packageName),
Version: string(pkgVer),
Types: inferredModule.SupportingTypes,
Provider: schema.ResourceSpec{
InputProperties: inferredModule.ProvidersConfig.Variables,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, we forgot this before.

},
Resources: map[string]schema.ResourceSpec{
mainResourceToken: {
IsComponent: true,
Expand Down
111 changes: 107 additions & 4 deletions pkg/modprovider/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
emptypb "google.golang.org/protobuf/types/known/emptypb"

"github.com/pulumi/pulumi/pkg/v3/resource/provider"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin"
"github.com/pulumi/pulumi/sdk/v3/go/common/tokens"
"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
Expand Down Expand Up @@ -61,6 +62,8 @@ type server struct {
packageVersion packageVersion
componentTypeName componentTypeName
inferredModuleSchema *InferredModuleSchema
providerConfig resource.PropertyMap
providerSelfURN pulumi.URN
}

func (s *server) Parameterize(
Expand Down Expand Up @@ -203,10 +206,23 @@ func (*server) GetPluginInfo(
}, nil
}

func (*server) Configure(
func (s *server) Configure(
_ context.Context,
_ *pulumirpc.ConfigureRequest,
req *pulumirpc.ConfigureRequest,
) (*pulumirpc.ConfigureResponse, error) {
config, err := plugin.UnmarshalProperties(req.Args, plugin.MarshalOptions{
KeepUnknowns: true,
RejectAssets: true,
KeepSecrets: true,
KeepOutputValues: false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What this does is rewires first-class outputs to Secret or Computed cases, but it also drops dependencies inherent in first-class outputs. Whether that messes things up for Pulumi or not, I am not sure. We could test this out later on. I'll add a ticket.

})

if err != nil {
return nil, fmt.Errorf("configure failed to parse inputs: %w", err)
}

s.providerConfig = config

return &pulumirpc.ConfigureResponse{
AcceptSecrets: true,
SupportsPreview: true,
Expand Down Expand Up @@ -264,6 +280,61 @@ func (s *server) acquirePackageReference(
return response.Ref, nil
}

// cleanProvidersConfig takes config that was produced from provider inputs in the program:
//
// const provider = new vpc.Provider("my-provider", {
// aws: {
// "region": "us-west-2"
// }
// })
//
// the input config here would look like sometimes where the provider config is a JSON string:
//
// {
// propertyKey("version"): stringProperty("0.1.0"),
// propertyKey("aws"): stringProperty("{\"region\": \"us-west-2\"}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain step by step, what's going on here? Are we affected by the JSON encoding again :/

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we affected by the JSON encoding again :/

Yes. Config values arrive to the provider JSON-stringified so here we change them back to an object format, if necessary. When they are already an object, we use that shape.

If the value is a string, then it must be a stringified JSON object since the schema for each field specifies a free-form object so it is same to assume.

// }
//
// notice how the value is a string that is a JSON stringified object due to legacy provider SDK behavior
// see https://github.com/pulumi/home/issues/3705 for reference
// we need to convert this to a map[string]resource.PropertyMap so that it can be used
// in the Terraform JSON file
func cleanProvidersConfig(config resource.PropertyMap) map[string]resource.PropertyMap {
providersConfig := make(map[string]resource.PropertyMap)
for propertyKey, serializedConfig := range config {
if string(propertyKey) == "version" || string(propertyKey) == "pluginDownloadURL" {
// skip the version and pluginDownloadURL properties
continue
}

if serializedConfig.IsString() {
value := serializedConfig.StringValue()
deserialized := map[string]interface{}{}
if err := json.Unmarshal([]byte(value), &deserialized); err != nil {
contract.Failf("failed to deserialize provider config into a map: %v", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this thinks if it's a string it's always JSON-encoded string. That's probably OK since the type of these values is map<any>.

}

if len(deserialized) > 0 {
providersConfig[string(propertyKey)] = resource.NewPropertyMapFromMap(deserialized)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing continue here? I don't think it should fall through to the next case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the continue so that the final fall through is a hard failure in case we get something we don't understand ✅

continue
}

if serializedConfig.IsObject() {
// we might later get the behaviour where all programs no longer send serialized JSON
// but send the actual object instead
// right now only YAML and Go programs send the actual object
// see https://github.com/pulumi/home/issues/3705 for reference
providersConfig[string(propertyKey)] = serializedConfig.ObjectValue()
continue
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's assert here if we fall through and we failed to parse it, we fail loud that we don't understand the encoding. This would be helpful weeding out some edge cases. I think secrets are encoded funny in JSON config encoding and we will need to tweak this a bit to fix that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done ✅


contract.Failf("cleanProvidersConfig failed to parse unsupported type: %v", serializedConfig)
}

return providersConfig
}

func (s *server) Construct(
ctx context.Context,
req *pulumirpc.ConstructRequest,
Expand All @@ -275,6 +346,8 @@ func (s *server) Construct(
// TODO[https://github.com/pulumi/pulumi-terraform-module/issues/151] support Outputs in Unmarshal
KeepOutputValues: false,
})

providersConfig := cleanProvidersConfig(s.providerConfig)
if err != nil {
return nil, fmt.Errorf("Construct failed to parse inputs: %s", err)
}
Expand Down Expand Up @@ -302,7 +375,9 @@ func (s *server) Construct(
name,
inputProps,
s.inferredModuleSchema,
packageRef)
packageRef,
s.providerSelfURN,
providersConfig)

if err != nil {
return nil, fmt.Errorf("NewModuleComponentResource failed: %w", err)
Expand Down Expand Up @@ -332,6 +407,33 @@ func (s *server) Check(
}
}

func (s *server) CheckConfig(
_ context.Context,
req *pulumirpc.CheckRequest,
) (*pulumirpc.CheckResponse, error) {
s.providerSelfURN = pulumi.URN(req.Urn)

config, err := plugin.UnmarshalProperties(req.GetNews(), plugin.MarshalOptions{
KeepUnknowns: true,
RejectAssets: true,
KeepSecrets: true,
KeepOutputValues: false,
})

if err != nil {
return nil, fmt.Errorf("CheckConfig failed to parse inputs: %w", err)
}

// keep provider config in memory for use later.
// we keep one instance of provider configuration because each configuration is used
// once per provider process.
s.providerConfig = config

return &pulumirpc.CheckResponse{
Inputs: req.News,
}, nil
}

func (s *server) Diff(
ctx context.Context,
req *pulumirpc.DiffRequest,
Expand Down Expand Up @@ -380,7 +482,8 @@ func (s *server) Delete(
) (*emptypb.Empty, error) {
switch {
case req.GetType() == string(moduleStateTypeToken(s.packageName)):
return s.moduleStateHandler.Delete(ctx, req, s.params.TFModuleSource, s.params.TFModuleVersion)
providersConfig := cleanProvidersConfig(s.providerConfig)
return s.moduleStateHandler.Delete(ctx, req, s.params.TFModuleSource, s.params.TFModuleVersion, providersConfig)
case isChildResourceType(req.GetType()):
return s.childHandler.Delete(ctx, req)
default:
Expand Down
21 changes: 21 additions & 0 deletions pkg/modprovider/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"google.golang.org/protobuf/types/known/emptypb"
"google.golang.org/protobuf/types/known/structpb"

"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource/urn"
"github.com/pulumi/pulumi/sdk/v3/go/common/tokens"
"github.com/pulumi/pulumi/sdk/v3/go/common/util/rpcutil"
Expand Down Expand Up @@ -298,3 +299,23 @@ func (s *testResourceMonitorServer) RegisterResourceOutputs(
func TestIsChildResourceType(t *testing.T) {
require.True(t, isChildResourceType("terraform-aws-modules:tf:aws_s3_bucket"))
}

func Test_cleanProvidersConfig(t *testing.T) {
inputConfig := resource.PropertyMap{
resource.PropertyKey("version"): resource.NewStringProperty("0.0.1"),
resource.PropertyKey("aws"): resource.NewStringProperty("{\"region\":\"us-west-2\"}"),
}

// cleaning the provider config in the form above is the what we get from Pulumi programs
// we clean it such that:
// - the version is removed
// - the provider configuration is parsed from the JSON string to a PropertyMap
cleaned := cleanProvidersConfig(inputConfig)
expected := map[string]resource.PropertyMap{
"aws": {
resource.PropertyKey("region"): resource.NewStringProperty("us-west-2"),
},
}

assert.Equal(t, expected, cleaned)
}
7 changes: 5 additions & 2 deletions pkg/modprovider/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ func (h *moduleStateHandler) Delete(
req *pulumirpc.DeleteRequest,
moduleSource TFModuleSource,
moduleVersion TFModuleVersion,
providersConfig map[string]resource.PropertyMap,
) (*emptypb.Empty, error) {
oldState := moduleState{}
oldState.Unmarshal(req.GetProperties())
Expand Down Expand Up @@ -273,6 +274,7 @@ func (h *moduleStateHandler) Delete(
tf.WorkingDir(),
olds["moduleInputs"].ObjectValue(), /*inputs*/
[]tfsandbox.TFOutputSpec{}, /*outputs*/
providersConfig,
)

if err != nil {
Expand Down Expand Up @@ -330,8 +332,9 @@ func (h *moduleStateHandler) Read(
// when refreshing, we do not require outputs to be exposed
err = tfsandbox.CreateTFFile(tfName, moduleSource, moduleVersion,
tf.WorkingDir(),
inputs, /*inputs*/
[]tfsandbox.TFOutputSpec{}, /*outputs*/
inputs, /*inputs*/
[]tfsandbox.TFOutputSpec{}, /*outputs*/
map[string]resource.PropertyMap{}, /*providersConfig*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a TODO here right? When read executes tofu refresh, it does need to have the provider config?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#186

)
if err != nil {
return nil, fmt.Errorf("Seed file generation failed: %w", err)
Expand Down
16 changes: 15 additions & 1 deletion pkg/modprovider/tfmodules.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type InferredModuleSchema struct {
Outputs map[string]schema.PropertySpec
SupportingTypes map[string]schema.ComplexTypeSpec
RequiredInputs []string
ProvidersConfig schema.ConfigSpec
}

var stringType = schema.TypeSpec{Type: "string"}
Expand Down Expand Up @@ -268,6 +269,18 @@ func InferModuleSchema(
Outputs: make(map[string]schema.PropertySpec),
RequiredInputs: []string{},
SupportingTypes: map[string]schema.ComplexTypeSpec{},
ProvidersConfig: schema.ConfigSpec{
Variables: map[string]schema.PropertySpec{},
},
}

if module.ProviderRequirements != nil {
for providerName := range module.ProviderRequirements.RequiredProviders {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable!

inferredModuleSchema.ProvidersConfig.Variables[providerName] = schema.PropertySpec{
Description: "provider configuration for " + providerName,
TypeSpec: mapType(anyType),
}
}
}

for variableName, variable := range module.Variables {
Expand Down Expand Up @@ -388,7 +401,8 @@ func resolveModuleSources(

inputs := resource.PropertyMap{}
outputs := []tfsandbox.TFOutputSpec{}
err = tfsandbox.CreateTFFile(key, source, version, tf.WorkingDir(), inputs, outputs)
providerConfig := map[string]resource.PropertyMap{}
err = tfsandbox.CreateTFFile(key, source, version, tf.WorkingDir(), inputs, outputs, providerConfig)
if err != nil {
return "", fmt.Errorf("tofu file creation failed: %w", err)
}
Expand Down
Loading
Loading