pulumi / pulumi

Pulumi - Infrastructure as Code in any programming language 🚀
https://www.pulumi.com
Apache License 2.0
21.22k stars 1.1k forks source link

Regression in the `@pulumi/pulumi` 3.128.0 for `PULUMI_CONFIG` env variable? #16974

Open ringods opened 1 month ago

ringods commented 1 month ago

What happened?

Customer reports they may have found a regression in the @pulumi/pulumi NPM package.

We were using 3.127.0 and tried updating to 3.129.0 this morning but all of our unit tests which set the config using process.env.PULUMI_CONFIG started failing with Missing required configuration variable xyz. Rolling back to 3.127.0 fixed the issue.

Example

No repro yet.

Output of pulumi about

@pulumi/pulumi v3.129.0

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

ringods commented 1 month ago

Customer was able to narrow it down to v3.128.0.

justinvp commented 1 month ago

I'm not seeing any obvious changes in @pulumi/pulumi between 3.127.0 and 3.128.0 that could impact this. Could they share a minimal snippet of a unit test that demonstrates the issue?

ringods commented 1 month ago

@justinvp the general setup of the customer their unit test code is like this:

import * as pulumi from '@pulumi/pulumi';
import { getDefaults } from '@customer-private-package';

process.env.PULUMI_CONFIG = JSON.stringify({
    'project:defaults': '{"foo":"bar"}',
});

pulumi.runtime.setMocks(
...
);

describe('Example', () => {
    it('should get the config', () => {
        const defaults = getDefaults();
        expect(defaults).toEqual({ foo: 'bar' });
    });
});

Since @pulumi/pulumi@v3.128.0, there is a cyclic import dependency between pulumi.runtime.state and pulumi.runtime.rpc, introduced in #13240:

The @customer-private-package uses code from the core Pulumi SDK. I haven't been able to pinpoint exactly which line it is, but I assume for now the cyclic dependency between runtime.state and runtime.rpc is what leads to a different evaluation order and more of the non-unittest code being loaded.

Supporting this assumption, I asked the customer to move the import of their own package after the setMocks invocation:

import * as pulumi from '@pulumi/pulumi';

process.env.PULUMI_CONFIG = JSON.stringify({
    'project:defaults': '{"foo":"bar"}',
});

pulumi.runtime.setMocks(
...
);

import { getDefaults } from '@customer-private-package';

describe('Example', () => {
    it('should get the config', () => {
        const defaults = getDefaults();
        expect(defaults).toEqual({ foo: 'bar' });
    });
});

This resolved the error they reported using @pulumi/pulumi v3.128.0.

justinvp commented 3 weeks ago

Thanks for the details @ringods! One more question: What does the implementation of getDefaults do?

justinvp commented 3 weeks ago

The import type should be erased at runtime, so it'd be interesting if this was indeed a cycle at runtime.

https://github.com/pulumi/pulumi/blob/4c0ccfa6ad6f4ce1191a6d649a561100f058d740/sdk/nodejs/runtime/state.ts#L22

justinvp commented 3 weeks ago

Another question @ringods: What are they using for the unit test? Sounds like possibly jest? Are they using ts-jest? I'm trying to put together a repro project, but haven't been able to reproduce the issue (works on 3.127.0 and latest 3.130.0).

ringods commented 3 weeks ago

The implementation of getDefaults is like this:

export interface Defaults {
  tags: {
    feature: string
    <plus other strings - no nested objects>
  }
}

export const getDefaults = () => {
  const config = new pulumi.Config();
  let defaults = config.getObject<Defaults>('defaultTags');
  if (defaults === undefined) {
    defaults = config.requireObject<Defaults>('defaults');
  }
  return defaults;
};

The test runner in use is ts-jest.

justinvp commented 2 weeks ago

I still can't reproduce this. I've created https://github.com/justinvp/pulumi-16974. When running the test, it succeeds. This is on macOS with Node v20.9.0.

@ringods, are you able to reproduce the regression?