Skip to content

Commit

Permalink
Maintain alias compat for older Node.js SDKs on new CLIs
Browse files Browse the repository at this point in the history
This change updates the engine to detect if a `RegisterResource` request
is coming from an older Node.js SDK that is using incorrect alias specs
and, if so, transforms the aliases to be correct. This allows us to
maintain compatibility for users who have upgraded their CLI but are
still using an older version of the Node.js SDK with incorrect alias
specs.

We detect if the request is from a Node.js SDK by looking at the gRPC
request's metadata headers, specifically looking at the "pulumi-runtime"
and "user-agent" headers.

First, if the request has a "pulumi-runtime" header with a value of
"nodejs", we know it's coming from the Node.js plugin. The Node.js
language plugin proxies gRPC calls from the Node.js SDK to the resource
monitor and the proxy now sets the "pulumi-runtime" header to "nodejs"
for `RegisterResource` calls.

Second, if the request has a "user-agent" header that starts with
"grpc-node-js/", we know it's coming from the Node.js SDK. This is the
case for inline programs in the automation API, which connects directly
to the resource monitor, rather than going through the language plugin's
proxy.

We can't just look at "user-agent", because in the proxy case it will
have a Go-specific "user-agent".

Updated Node.js SDKs set a new `aliasSpecs` field on the
`RegisterResource` request, which indicates that the alias specs are
correct, and no transforms are needed.
  • Loading branch information
justinvp committed Jun 14, 2023
1 parent c7e0e3d commit 1026e7b
Show file tree
Hide file tree
Showing 11 changed files with 355 additions and 27 deletions.
93 changes: 93 additions & 0 deletions pkg/engine/lifecycletest/pulumi_test.go
Expand Up @@ -1473,6 +1473,9 @@ type Resource struct {
dependencies []resource.URN
parent resource.URN
deleteBeforeReplace bool

aliasSpecs bool
grpcRequestHeaders map[string]string
}

func registerResources(t *testing.T, monitor *deploytest.ResourceMonitor, resources []Resource) error {
Expand All @@ -1484,6 +1487,8 @@ func registerResources(t *testing.T, monitor *deploytest.ResourceMonitor, resour
DeleteBeforeReplace: &r.deleteBeforeReplace,
AliasURNs: r.aliasURNs,
Aliases: r.aliases,
AliasSpecs: r.aliasSpecs,
GrpcRequestHeaders: r.grpcRequestHeaders,
})
if err != nil {
return err
Expand Down Expand Up @@ -2002,6 +2007,94 @@ func TestAliases(t *testing.T) {
}}, []display.StepOp{deploy.OpSame}, false)
}

func TestAliasesNodeJSBackCompat(t *testing.T) {
t.Parallel()

loaders := []*deploytest.ProviderLoader{
deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
return &deploytest.Provider{}, nil
}),
}

updateProgramWithResource := createUpdateProgramWithResourceFuncForAliasTests(t, loaders)

tests := []struct {
name string
aliasSpecs bool
grpcRequestHeaders map[string]string
noParentAlias resource.Alias
}{
{
name: "Old Node SDK",
grpcRequestHeaders: map[string]string{"pulumi-runtime": "nodejs"},
// Old Node.js SDKs set Parent to "" rather than setting NoParent to true,
noParentAlias: resource.Alias{Parent: ""},
},
{
name: "New Node SDK",
grpcRequestHeaders: map[string]string{"pulumi-runtime": "nodejs"},
// Indicate we're sending alias specs correctly.
aliasSpecs: true,
// Properly set NoParent to true.
noParentAlias: resource.Alias{NoParent: true},
},
{
name: "Unknown SDK",
// Properly set NoParent to true.
noParentAlias: resource.Alias{NoParent: true},
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

snap := updateProgramWithResource(nil, []Resource{{
t: "pkgA:index:t1",
name: "one",
aliasSpecs: tt.aliasSpecs,
grpcRequestHeaders: tt.grpcRequestHeaders,
}, {
t: "pkgA:index:t2",
name: "two",
aliasSpecs: tt.aliasSpecs,
grpcRequestHeaders: tt.grpcRequestHeaders,
}}, []display.StepOp{deploy.OpCreate}, false)

// Now make "two" a child of "one" ensuring no changes.
snap = updateProgramWithResource(snap, []Resource{{
t: "pkgA:index:t1",
name: "one",
aliasSpecs: tt.aliasSpecs,
grpcRequestHeaders: tt.grpcRequestHeaders,
}, {
t: "pkgA:index:t2",
parent: "urn:pulumi:test::test::pkgA:index:t1::one",
name: "two",
aliases: []resource.Alias{
tt.noParentAlias,
},
aliasSpecs: tt.aliasSpecs,
grpcRequestHeaders: tt.grpcRequestHeaders,
}}, []display.StepOp{deploy.OpSame}, false)

// Now remove the parent relationship.
_ = updateProgramWithResource(snap, []Resource{{
t: "pkgA:index:t1",
name: "one",
}, {
t: "pkgA:index:t2",
name: "two",
aliases: []resource.Alias{
{Parent: "urn:pulumi:test::test::pkgA:index:t1::one"},
},
aliasSpecs: tt.aliasSpecs,
grpcRequestHeaders: tt.grpcRequestHeaders,
}}, []display.StepOp{deploy.OpSame}, false)
})
}
}

func TestAliasURNs(t *testing.T) {
t.Parallel()

Expand Down
11 changes: 10 additions & 1 deletion pkg/resource/deploy/deploytest/resourcemonitor.go
Expand Up @@ -27,6 +27,7 @@ import (
pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/metadata"
)

type ResourceMonitor struct {
Expand Down Expand Up @@ -108,9 +109,11 @@ type ResourceOptions struct {
Remote bool
Providers map[string]string
AdditionalSecretOutputs []resource.PropertyKey
AliasSpecs bool

DisableSecrets bool
DisableResourceReferences bool
GrpcRequestHeaders map[string]string
}

func (rm *ResourceMonitor) RegisterResource(t tokens.Type, name string, custom bool,
Expand Down Expand Up @@ -228,10 +231,16 @@ func (rm *ResourceMonitor) RegisterResource(t tokens.Type, name string, custom b
AdditionalSecretOutputs: additionalSecretOutputs,
Aliases: aliasObjects,
DeletedWith: string(opts.DeletedWith),
AliasSpecs: opts.AliasSpecs,
}

ctx := context.Background()
if len(opts.GrpcRequestHeaders) > 0 {
ctx = metadata.NewOutgoingContext(ctx, metadata.New(opts.GrpcRequestHeaders))
}

// submit request
resp, err := rm.resmon.RegisterResource(context.Background(), requestInput)
resp, err := rm.resmon.RegisterResource(ctx, requestInput)
if err != nil {
return "", "", nil, err
}
Expand Down
50 changes: 50 additions & 0 deletions pkg/resource/deploy/source_eval.go
Expand Up @@ -30,6 +30,7 @@ import (

"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/metadata"

"github.com/pulumi/pulumi/pkg/v3/resource/deploy/providers"
interceptors "github.com/pulumi/pulumi/pkg/v3/util/rpcdebug"
Expand Down Expand Up @@ -1006,6 +1007,45 @@ func inheritFromParent(child resource.Goal, parent resource.Goal) *resource.Goal
return &goal
}

// requestFromNodeJS returns true if the request is coming from a Node.js language runtime
// or SDK. This is determined by checking if the request has a "pulumi-runtime" metadata
// header with a value of "nodejs". If no "pulumi-runtime" header is present, then it
// checks if the request has a "user-agent" metadata header that has a value that starts
// with "grpc-node-js/".
func requestFromNodeJS(ctx context.Context) bool {
if md, hasMetadata := metadata.FromIncomingContext(ctx); hasMetadata {
// Check for the "pulumi-runtime" header first.
// We'll always respect this header value when present.
if runtime, ok := md["pulumi-runtime"]; ok {
return len(runtime) == 1 && runtime[0] == "nodejs"
}
// Otherwise, check the "user-agent" header.
if ua, ok := md["user-agent"]; ok {
return len(ua) == 1 && strings.HasPrefix(ua[0], "grpc-node-js/")
}
}
return false
}

// transformAliasForNodeJSCompat transforms the alias from the legacy Node.js values to properly specified values.
func transformAliasForNodeJSCompat(alias resource.Alias) resource.Alias {
contract.Assertf(alias.URN == "", "alias.URN must be empty")
// The original implementation in the Node.js SDK did not specify aliases correctly:
//
// - It did not set NoParent when it should have, but instead set Parent to empty.
// - It set NoParent to true and left Parent empty when both the alias and resource had no Parent specified.
//
// To maintain compatibility with such versions of the Node.js SDK, we transform these incorrectly
// specified aliases into properly specified ones that work with this implementation of the engine:
//
// - { Parent: "", NoParent: false } -> { Parent: "", NoParent: true }
// - { Parent: "", NoParent: true } -> { Parent: "", NoParent: false }
if alias.Parent == "" {
alias.NoParent = !alias.NoParent
}
return alias
}

// RegisterResource is invoked by a language process when a new resource has been allocated.
func (rm *resmon) RegisterResource(ctx context.Context,
req *pulumirpc.RegisterResourceRequest,
Expand Down Expand Up @@ -1088,6 +1128,13 @@ func (rm *resmon) RegisterResource(ctx context.Context,
aliases = append(aliases, resource.Alias{URN: resource.URN(aliasURN)})
}

// We assume aliases are properly specified. However, if a request hasn't explicitly
// indicated that it is using properly specified aliases and the request is coming
// from Node.js, transform the aliases from the incorrect Node.js values to properly
// specified values, to maintain backward compatibility for users of older Node.js
// SDKs that aren't sending properly specified aliases.
transformAliases := !req.GetAliasSpecs() && requestFromNodeJS(ctx)

for _, aliasObject := range req.GetAliases() {
aliasSpec := aliasObject.GetSpec()
var alias resource.Alias
Expand All @@ -1100,6 +1147,9 @@ func (rm *resmon) RegisterResource(ctx context.Context,
Parent: resource.URN(aliasSpec.GetParentUrn()),
NoParent: aliasSpec.GetNoParent(),
}
if transformAliases {
alias = transformAliasForNodeJSCompat(alias)
}
} else {
alias = resource.Alias{
URN: resource.URN(aliasObject.GetUrn()),
Expand Down
120 changes: 120 additions & 0 deletions pkg/resource/deploy/source_eval_test.go
Expand Up @@ -25,6 +25,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/metadata"

"github.com/pulumi/pulumi/pkg/v3/resource/deploy/deploytest"
"github.com/pulumi/pulumi/pkg/v3/resource/deploy/providers"
Expand Down Expand Up @@ -1164,3 +1165,122 @@ func TestResourceInheritsOptionsFromParent(t *testing.T) {
})
}
}

func TestRequestFromNodeJS(t *testing.T) {
t.Parallel()

ctx := context.Background()
newContext := func(md map[string]string) context.Context {
return metadata.NewIncomingContext(ctx, metadata.New(md))
}

tests := []struct {
name string
ctx context.Context
expected bool
}{
{
name: "no metadata",
ctx: ctx,
expected: false,
},
{
name: "empty metadata",
ctx: newContext(map[string]string{}),
expected: false,
},
{
name: "user-agent foo/1.0",
ctx: newContext(map[string]string{"user-agent": "foo/1.0"}),
expected: false,
},
{
name: "user-agent grpc-node-js/1.8.15",
ctx: newContext(map[string]string{"user-agent": "grpc-node-js/1.8.15"}),
expected: true,
},
{
name: "pulumi-runtime foo",
ctx: newContext(map[string]string{"pulumi-runtime": "foo"}),
expected: false,
},
{
name: "pulumi-runtime nodejs",
ctx: newContext(map[string]string{"pulumi-runtime": "nodejs"}),
expected: true,
},
{
// Always respect the value of pulumi-runtime, regardless of the user-agent.
name: "user-agent grpc-go/1.54.0, pulumi-runtime nodejs",
ctx: newContext(map[string]string{
"user-agent": "grpc-go/1.54.0",
"pulumi-runtime": "nodejs",
}),
expected: true,
},
{
name: "user-agent grpc-node-js/1.8.15, pulumi-runtime python",
ctx: newContext(map[string]string{
"user-agent": "grpc-node-js/1.8.15",
"pulumi-runtime": "python",
}),
expected: false,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
actual := requestFromNodeJS(tt.ctx)
assert.Equal(t, tt.expected, actual)
})
}
}

func TestTransformAliasForNodeJSCompat(t *testing.T) {
t.Parallel()
tests := []struct {
name string
input resource.Alias
expected resource.Alias
}{
{
name: `{Parent: "", NoParent: true} (transformed)`,
input: resource.Alias{Parent: "", NoParent: true},
expected: resource.Alias{Parent: "", NoParent: false},
},
{
name: `{Parent: "", NoParent: false} (transformed)`,
input: resource.Alias{Parent: "", NoParent: false},
expected: resource.Alias{Parent: "", NoParent: true},
},
{
name: `{Parent: "", NoParent: false, Name: "name"} (transformed)`,
input: resource.Alias{Parent: "", NoParent: false, Name: "name"},
expected: resource.Alias{Parent: "", NoParent: true, Name: "name"},
},
{
name: `{Parent: "", NoParent: true, Name: "name"} (transformed)`,
input: resource.Alias{Parent: "", NoParent: true, Name: "name"},
expected: resource.Alias{Parent: "", NoParent: false, Name: "name"},
},
{
name: `{Parent: "foo", NoParent: false} (no transform)`,
input: resource.Alias{Parent: "foo", NoParent: false},
expected: resource.Alias{Parent: "foo", NoParent: false},
},
{
name: `{Parent: "foo", NoParent: false, Name: "name"} (no transform)`,
input: resource.Alias{Parent: "foo", NoParent: false, Name: "name"},
expected: resource.Alias{Parent: "foo", NoParent: false, Name: "name"},
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
actual := transformAliasForNodeJSCompat(tt.input)
assert.Equal(t, tt.expected, actual)
})
}
}
9 changes: 9 additions & 0 deletions proto/pulumi/resource.proto
Expand Up @@ -118,6 +118,15 @@ message RegisterResourceRequest {
bool retainOnDelete = 25; // if true the engine will not call the resource providers delete method for this resource.
repeated Alias aliases = 26; // a list of additional aliases that should be considered the same.
string deletedWith = 27; // if set the engine will not call the resource providers delete method for this resource when specified resource is deleted.

// Indicates that alias specs are specified correctly according to the spec.
// Older versions of the Node.js SDK did not send alias specs correctly.
// If this is not set to true and the engine detects the request is from the
// Node.js runtime, the engine will transform incorrect alias specs into
// correct ones.
// Other SDKs that are correctly specifying alias specs could set this to
// true, but it's not necessary.
bool aliasSpecs = 28;
}

// RegisterResourceResponse is returned by the engine after a resource has finished being initialized. It includes the
Expand Down
2 changes: 2 additions & 0 deletions sdk/nodejs/cmd/pulumi-language-nodejs/proxy.go
Expand Up @@ -23,6 +23,7 @@ import (

"google.golang.org/grpc/encoding"
"google.golang.org/grpc/encoding/proto"
"google.golang.org/grpc/metadata"

"github.com/pulumi/pulumi/sdk/v3/go/common/util/logging"
pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
Expand Down Expand Up @@ -231,6 +232,7 @@ func (p *monitorProxy) ReadResource(
func (p *monitorProxy) RegisterResource(
ctx context.Context, req *pulumirpc.RegisterResourceRequest,
) (*pulumirpc.RegisterResourceResponse, error) {
ctx = metadata.AppendToOutgoingContext(ctx, "pulumi-runtime", "nodejs")
return p.target.RegisterResource(ctx, req)
}

Expand Down

0 comments on commit 1026e7b

Please sign in to comment.