Fix PULUMI_GCP_DISABLE_GLOBAL_PROJECT_WARNING/gcp:disableGlobalProjec…

…tWarning (#2831)

The current "disableGlobalProjectWarning" check happens after the global
project warning has already been displayed, and instead serves to skip
the gcloud config validation. This PR moves the check to the correct
place, and puts in tests on the logged output.

Fixes #2829

.ci-mgmt.yaml

@@ -31,10 +31,6 @@ plugins:
     version: "1.0.17"
     kind: converter
 goBuildParallelism: 2
-actions:
-  preTest:
-    - name: Run provider tests
-      run: |
-        cd provider && go test -v -count=1 -cover -timeout 2h -tags=${{ matrix.language }} -parallel 4 .
 # Use `pulumi convert` for translating examples from TF to Pulumi.
 pulumiConvert: 1
+integrationTestProvider: true

.github/workflows/test.yml

@@ -98,8 +98,9 @@ jobs:
         token: ${{ secrets.GITHUB_TOKEN }}
         version: v2.5.0
     - name: Run provider tests
-      run: |
-        cd provider && go test -v -count=1 -cover -timeout 2h -tags=${{ matrix.language }} -parallel 4 .
+      if: matrix.testTarget == 'local'
+      working-directory: provider
+      run: go test -v -count=1 -cover -timeout 2h -tags=${{ matrix.language }} -parallel 4 .
     - name: Run tests
       if: matrix.testTarget == 'local'
       run: cd examples && go test -v -count=1 -cover -timeout 2h -tags=${{ matrix.language }} -skip TestPulumiExamples -parallel 4 .

provider/gcp_test.go

@@ -8,9 +8,14 @@ import (
 	"sync/atomic"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/api/compute/v1"
 	"google.golang.org/api/option"
+
+	"github.com/pulumi/pulumi-terraform-bridge/v3/unstable/testutil"
+	"github.com/pulumi/pulumi/sdk/v3/go/common/diag"
+	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 )
 
 func RegionListReturnHandler(regions []string) func(http.ResponseWriter, *http.Request) {
@@ -70,7 +75,9 @@ func TestPreConfigureCallbackNoErrWhenRegionDifferent(t *testing.T) {
 		},
 	)
 
-	err := preConfigureCall(context.Background(), nil, nil, nil)
+	ctx := testutil.InitLogging(t, context.Background(), nil)
+
+	err := preConfigureCall(ctx, nil, nil, nil)
 	require.NoError(t, err)
 }
 
@@ -122,6 +129,104 @@ func TestPreConfigureCallbackNoErrWhenRegionListCallErrors(t *testing.T) {
 			option.WithoutAuthentication(), option.WithEndpoint(ts.URL),
 		},
 	)
-	err := preConfigureCall(context.Background(), nil, nil, nil)
+	ctx := testutil.InitLogging(t, context.Background(), nil)
+	err := preConfigureCall(ctx, nil, nil, nil)
 	require.NoError(t, err)
 }
+
+func TestPreConfigureCallbackNoWarningWhenNoProjectEnvSet(t *testing.T) {
+	if testing.Short() {
+		t.Skipf("Skipping in testing.Short() mode, assuming this is a CI run without GCP creds")
+	}
+
+	expectedLogs := &expectLogs{t, nil}
+	defer expectedLogs.assertDone()
+
+	ctx := testutil.InitLogging(t, context.Background(), expectedLogs)
+	callback := preConfigureCallbackWithLogger(new(atomic.Bool), nil)
+
+	t.Setenv("PULUMI_GCP_DISABLE_GLOBAL_PROJECT_WARNING", "true")
+
+	assert.NoError(t, callback(ctx, nil, nil, nil))
+}
+
+func TestPreConfigureCallbackNoWarningWhenNoProjectConfigSet(t *testing.T) {
+	t.Parallel()
+	if testing.Short() {
+		t.Skipf("Skipping in testing.Short() mode, assuming this is a CI run without GCP creds")
+	}
+
+	expectedLogs := &expectLogs{t, nil}
+	defer expectedLogs.assertDone()
+
+	ctx := testutil.InitLogging(t, context.Background(), expectedLogs)
+	callback := preConfigureCallbackWithLogger(new(atomic.Bool), nil)
+
+	assert.NoError(t, callback(ctx, nil, resource.PropertyMap{
+		"disableGlobalProjectWarning": resource.NewProperty(true),
+	}, nil))
+}
+
+func TestPreConfigureCallbackNoWarningWhenProjectSet(t *testing.T) {
+	t.Parallel()
+	if testing.Short() {
+		t.Skipf("Skipping in testing.Short() mode, assuming this is a CI run without GCP creds")
+	}
+
+	expectedLogs := &expectLogs{t, nil}
+	defer expectedLogs.assertDone()
+
+	ctx := testutil.InitLogging(t, context.Background(), expectedLogs)
+	callback := preConfigureCallbackWithLogger(new(atomic.Bool), nil)
+
+	assert.NoError(t, callback(ctx, nil, resource.PropertyMap{
+		"project":              resource.NewProperty("my-project"),
+		"skipRegionValidation": resource.NewProperty(true),
+	}, nil))
+}
+
+func TestPreConfigureCallbackWarningWhenNoProject(t *testing.T) {
+	if testing.Short() {
+		t.Skipf("Skipping in testing.Short() mode, assuming this is a CI run without GCP creds")
+	}
+	expectedLogs := &expectLogs{t, []string{noProjectErr}}
+	defer expectedLogs.assertDone()
+
+	t.Setenv("GOOGLE_PROJECT", "")
+
+	ctx := testutil.InitLogging(t, context.Background(), expectedLogs)
+	callback := preConfigureCallbackWithLogger(new(atomic.Bool), nil)
+
+	assert.NoError(t, callback(ctx, nil, nil, nil))
+}
+
+type expectLogs struct {
+	t *testing.T
+
+	expected []string
+}
+
+func (e *expectLogs) Log(_ context.Context, sev diag.Severity, urn resource.URN, msg string) error {
+	assert.Equal(e.t, diag.Warning, sev, "Expect all logs to be warnings")
+	assert.Empty(e.t, urn, "Expected an empty URN for provider logs")
+
+	if len(e.expected) == 0 {
+		assert.Fail(e.t, "No logs were expected")
+	} else if e.expected[0] == msg {
+		e.expected = e.expected[1:] // Pop the message from the expected stack
+	} else {
+		assert.Failf(e.t, "unexpected log", "%q", msg)
+	}
+	return nil
+}
+func (e *expectLogs) LogStatus(_ context.Context, sev diag.Severity, urn resource.URN, msg string) error {
+	assert.Failf(e.t, "Unexpected status", "log: %s@%s: %q", sev, urn, msg)
+	return nil
+}
+
+func (e *expectLogs) assertDone() {
+	if len(e.expected) == 0 {
+		return
+	}
+	assert.Fail(e.t, "un-popped logs found", "logs: %#v", e.expected)
+}

provider/resources.go

@@ -5,7 +5,6 @@ package gcp
 import (
 	"context"
 	"fmt"
-	"log"
 	"path"
 	"strings"
 	"sync/atomic"
@@ -29,7 +28,6 @@ import (
 	shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2"
 	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/walk"
 	"github.com/pulumi/pulumi/pkg/v3/resource/provider"
-	"github.com/pulumi/pulumi/sdk/v3/go/common/diag"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/tokens"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
@@ -377,32 +375,28 @@ var wrongRegionErr string
 //go:embed errors/no_project.txt
 var noProjectErr string
 
-func logOrPrint(ctx context.Context, host *provider.HostClient, msg string) {
-	// host is unavailable in tests, so we revert to normal logging.
-	if host != nil {
-		// the URN will default to the root stack name which is exactly what we want
-		_ = host.Log(ctx, diag.Warning, "", msg)
-	} else {
-		log.Print(msg)
-	}
-}
-
 // gcpClientOpts is used to pass in options during testing.
-func preConfigureCallbackWithLogger(credentialsValidationRun *atomic.Bool, gcpClientOpts []option.ClientOption) func(
-	ctx context.Context, host *provider.HostClient, vars resource.PropertyMap, c shim.ResourceConfig,
-) error {
-	return func(ctx context.Context, host *provider.HostClient, vars resource.PropertyMap, _ shim.ResourceConfig) error {
+func preConfigureCallbackWithLogger(
+	credentialsValidationRun *atomic.Bool, gcpClientOpts []option.ClientOption,
+) tfbridge.PreConfigureCallbackWithLogger {
+	return func(ctx context.Context, _ *provider.HostClient, vars resource.PropertyMap, _ shim.ResourceConfig) error {
 		if !credentialsValidationRun.CompareAndSwap(false, true) {
 			return nil
 		}
+
 		project := tfbridge.ConfigStringValue(vars, "project", []string{
 			"GOOGLE_PROJECT",
 			"GOOGLE_CLOUD_PROJECT",
 			"GCLOUD_PROJECT",
 			"CLOUDSDK_CORE_PROJECT",
 		})
 		if project == "" {
-			logOrPrint(ctx, host, noProjectErr)
+			if !tfbridge.ConfigBoolValue(
+				vars, "disableGlobalProjectWarning",
+				[]string{"PULUMI_GCP_DISABLE_GLOBAL_PROJECT_WARNING"},
+			) {
+				tfbridge.GetLogger(ctx).Warn(noProjectErr)
+			}
 			return nil
 		}
 
@@ -433,14 +427,6 @@ func preConfigureCallbackWithLogger(credentialsValidationRun *atomic.Bool, gcpCl
 			vars, "skipRegionValidation", []string{"PULUMI_GCP_SKIP_REGION_VALIDATION"},
 		)
 
-		disableGlobalProjectWarning := tfbridge.ConfigBoolValue(
-			vars, "disableGlobalProjectWarning", []string{"PULUMI_GCP_DISABLE_GLOBAL_PROJECT_WARNING"},
-		)
-
-		if disableGlobalProjectWarning {
-			return nil
-		}
-
 		// validate the gcloud config
 		err := config.LoadAndValidate(ctx)
 		if err != nil {
@@ -450,15 +436,15 @@ func preConfigureCallbackWithLogger(credentialsValidationRun *atomic.Bool, gcpCl
 		if !skipRegionValidation && config.Region != "" && config.Project != "" {
 			regionList, err := getRegionsList(ctx, config.Project, gcpClientOpts)
 			if err != nil {
-				logOrPrint(ctx, host, fmt.Sprintf("failed to get regions list: %v", err))
+				tfbridge.GetLogger(ctx).Warn(fmt.Sprintf("failed to get regions list: %v", err))
 				return nil
 			}
 			for _, region := range regionList {
 				if region == config.Region {
 					return nil
 				}
 			}
-			logOrPrint(ctx, host, fmt.Sprintf(wrongRegionErr, config.Region, config.Project))
+			tfbridge.GetLogger(ctx).Warn(fmt.Sprintf(wrongRegionErr, config.Region, config.Project))
 		}
 
 		return nil