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.

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

Adding in support for a "No Project" setting for gcp:project #2653

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion provider/errors/no_project.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
unable to detect a global setting for GCP Project.
Pulumi will rely on per-resource settings for this operation.
Set the GCP Project by using:
`pulumi config set gcp:project <project>`
`pulumi config set gcp:project <project>`
If you would like to override global project setting:
`pulumi config set gcp:project DisableGlobalProjectWarning`
15 changes: 15 additions & 0 deletions provider/provider_yaml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,21 @@ func TestNoGlobalProjectWarning(t *testing.T) {
)
}

func TestGlobalProjectNoProjectWarning(t *testing.T) {
if testing.Short() {
t.Skipf("Skipping in testing.Short() mode, assuming this is a CI run without GCP creds")
}
cwd, err := os.Getwd()
require.NoError(t, err)
proj := getProject()
test := pulumitest.NewPulumiTest(t, "test-programs/project-bucket",
opttest.LocalProviderPath(providerName, filepath.Join(cwd, "..", "bin")))

test.SetConfig(t, "gcp:project", noProjectMarker)
test.SetConfig(t, "gcpProj", proj)
test.Up(t)
}

// Test programs that were automatically extracted from examples without autocorrection.
func TestAutoExtractedProgramsUpgrade(t *testing.T) {
type testCase struct {
Expand Down
23 changes: 12 additions & 11 deletions provider/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,10 @@ const (
gcpWorkstations = "Workstations" // Workstations
)

// When `gcp:project` is set to noProjectMarker, we don't warn that a global project is missing
// and we don't try to validate the project.
const noProjectMarker = "DisableGlobalProjectWarning"
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit late to the party here but should we perhaps add a new config to disable the warning instead? That seems like a nicer UX for users than a magic string in place of the project config and a bit easier to discover.

e.g.

So instead of:

gcp.Provider(project="DisableGlobalProjectWarning")

users would use

gcp.Provider(disable_global_project_warning=true)

You can define that here:

"skipRegionValidation": {

We have an existing config we've added to disable another similar check.

Copy link
Contributor

@VenelinMartinov VenelinMartinov Nov 20, 2024

Choose a reason for hiding this comment

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

The benefit of this is that it would automatically get added to our docs etc. It would also be discoverable as a property on the ProviderArgs.

It might also work better with other things in the provider as I do not know all the places where the project is used - setting it to an invalid string might interact in odd ways with other things.

Copy link
Contributor

Choose a reason for hiding this comment

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

@rshade LMK what you think, also happy to help with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iwahbe @guineveresaenger thoughts? I am good any way we do it.

Copy link
Member

Choose a reason for hiding this comment

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

This is definitely better.

gcp.Provider(disable_global_project_warning=true)

When I saw DisableGlobalProjectWarning I initially assumed that ☝️ is what was meant (which I then promptly forgot to right down when I saw the code ☹️).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

excellent thank you @VenelinMartinov @iwahbe I will get that put in on Monday.


var moduleMapping = map[string]string{
"access_approval": gcpAccessApproval,
"access_context_manager": gcpAccessContextManager,
Expand Down Expand Up @@ -414,12 +418,7 @@ func preConfigureCallbackWithLogger(credentialsValidationRun *atomic.Bool, gcpCl
}),
ImpersonateServiceAccount: tfbridge.ConfigStringValue(vars,
"impersonateServiceAccount", []string{"GOOGLE_IMPERSONATE_SERVICE_ACCOUNT"}),
Project: tfbridge.ConfigStringValue(vars, "project", []string{
"GOOGLE_PROJECT",
"GOOGLE_CLOUD_PROJECT",
"GCLOUD_PROJECT",
"CLOUDSDK_CORE_PROJECT",
}),
Project: project,
Region: tfbridge.ConfigStringValue(vars, "region", []string{
"GOOGLE_REGION",
"GCLOUD_REGION",
Expand All @@ -433,19 +432,21 @@ func preConfigureCallbackWithLogger(credentialsValidationRun *atomic.Bool, gcpCl
}

// validate the gcloud config
err := config.LoadAndValidate(ctx)
if err != nil {
return fmt.Errorf(noCredentialsErr, err)
if config.Project != noProjectMarker {
err := config.LoadAndValidate(ctx)
if err != nil {
return fmt.Errorf(noCredentialsErr, err)
}
}

skipRegionValidation := tfbridge.ConfigBoolValue(
vars, "skipRegionValidation", []string{"PULUMI_GCP_SKIP_REGION_VALIDATION"},
)

if !skipRegionValidation && config.Region != "" && config.Project != "" {
if !skipRegionValidation && config.Region != "" && config.Project != "" && config.Project != noProjectMarker {
regionList, err := getRegionsList(ctx, config.Project, gcpClientOpts)
if err != nil {
logOrPrint(ctx, host, fmt.Sprintf("failed to get regions list: %v", err))
logOrPrint(ctx, host, fmt.Sprintf("failed to get regions list for project(%s): %v", config.Project, err))
return nil
}
for _, region := range regionList {
Expand Down
Loading