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

Do not error on duplicate config keys #402

Merged
merged 12 commits into from Nov 8, 2022
Merged

Do not error on duplicate config keys #402

merged 12 commits into from Nov 8, 2022

Conversation

aq17
Copy link
Contributor

@aq17 aq17 commented Nov 7, 2022

Fixes #401

@aq17 aq17 requested review from AaronFriel, iwahbe and Frassle and removed request for iwahbe November 7, 2022 18:28
pkg/pulumiyaml/sort.go Outdated Show resolved Hide resolved
@aq17 aq17 requested a review from AaronFriel November 8, 2022 19:18
@@ -97,7 +97,8 @@ func TestGenerateExamples(t *testing.T) {
_, template, diags, err := codegen.LoadTemplate(exampleProjectDir)
require.NoError(t, err, "Loading project %v", dir)
require.False(t, diags.HasErrors(), diags.Error())
assert.Len(t, diags, 0, "Should have neither warnings nor errors")
// TODO: update examples to use `config` instead of `configuration`. For now, allow the "`configuration` field is deprecated" warning.
// assert.Len(t, diags, 0, "Should have neither warnings nor errors")
Copy link
Member

Choose a reason for hiding this comment

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

Can we change this to a loop to filter out diags containing "`configuration` field is deprecated"?

Copy link
Member

@AaronFriel AaronFriel left a comment

Choose a reason for hiding this comment

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

LGTM!

@AaronFriel AaronFriel merged commit e1986dd into main Nov 8, 2022
@AaronFriel AaronFriel deleted the aqiu/401 branch November 8, 2022 22:05
@joeduffy
Copy link
Member

@aq17 @AaronFriel Glad to see a test in here, just wasn't immediately obvious it was protecting against the duplicate config key issue that led to the problem. Does it cover that? If yes, great! If not, should we be adding a new test?

@AaronFriel
Copy link
Member

It does & this PR removes the failure condition you found in #401.

There are two follow ups here:

cd $(mktemp -d)

/tmp/tmp.qP70vtV5Ub 
❯ pulumi new kubernetes-gcp-yaml -y

Created project 'tmp.qP70vtV5Ub'

Please enter your desired stack name.
To create a stack in an organization, use the format <org-name>/<stack-name> (e.g. `acmecorp/dev`).
Created stack 'dev'

Saved config

Your new project is ready to go! 

To perform an initial deployment, run `pulumi up`

/tmp/tmp.qP70vtV5Ub 
❯ pulumi preview
Previewing update (dev)

View Live: https://app.pulumi.com/friel/tmp.qP70vtV5Ub/dev/previews/edac5652-3dbc-4f18-bbfe-e7ca55a5c84f

     Type                           Name                Plan       Info
 +   pulumi:pulumi:Stack            tmp.qP70vtV5Ub-dev  create     1 warning; 1 message
 +   ├─ gcp:compute:Network         gke-network         create     
 +   ├─ gcp:compute:Subnetwork      gke-subnet          create     
 +   ├─ gcp:container:Cluster       gke-cluster         create     
 +   ├─ gcp:serviceAccount:Account  gke-nodepool-sa     create     
 +   └─ gcp:container:NodePool      gke-nodepool        create     


Diagnostics:
  pulumi:pulumi:Stack (tmp.qP70vtV5Ub-dev):
    Warning: Pulumi.yaml: root-level `configuration` field is deprecated; please use `config` instead.

    warning: 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>`

Outputs:
    clusterId  : output<string>
    clusterName: "gke-cluster-2792fcd"
    kubeconfig : [secret]
    networkId  : output<string>
    networkName: "gke-network-f68e324"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate config errors in templates
3 participants