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 f5b1175
Show file tree
Hide file tree
Showing 12 changed files with 374 additions and 28 deletions.
93 changes: 93 additions & 0 deletions pkg/engine/lifecycletest/pulumi_test.go
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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)
})
}
}
2 changes: 1 addition & 1 deletion proto/.checksum.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@
2284604767 7368 proto/pulumi/language.proto
2700626499 1743 proto/pulumi/plugin.proto
164600211 22361 proto/pulumi/provider.proto
1325776472 11014 proto/pulumi/resource.proto
3734563396 11493 proto/pulumi/resource.proto
9 changes: 9 additions & 0 deletions proto/pulumi/resource.proto
Original file line number Diff line number Diff line change
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

0 comments on commit f5b1175

Please sign in to comment.