Hierarchical config: optional typing extended short-hand syntax #11192

Zaid-Ajaj · 2022-10-30T00:13:48Z

Description

This PR extends the specs of the hierarchical configuration rules in the following manner:

The type property/attribute is made optional for project configuration which means stack values overriding this config block will not be validated against the type (integer stack value will override string project value)
Stacks can now use non-project config values that don't have to be defined at the project level (because they are not namespaced by the project, i.e. aws:region)
Non-project config values (i.e. aws:region) defined at the project level cannot have a type nor default properties, only value
Project config block using short-hand syntax now accept arrays: pulumi:disable-default-providers: ["*"]
Project config block when defined using value can be anything (objects, arrays, primitives)
Project config blocks cannot have both value and default defined at the same time

Fixes #11127
Fixes #11128

See added tests for more details

Checklist

I have added tests that prove my fix is effective or that my feature works

I have run make changelog and committed the changelog/pending/<file> documenting my change

Yes, there are changes in this PR that warrants bumping the Pulumi Service API version

pulumi-bot · 2022-10-30T00:14:39Z

Changelog

[uncommitted] (2022-10-31)

Features

[cli/config] Typing made optional, extended short-hand values to arrays and correctly pass stack name to config validator
#11192

Frassle · 2022-10-30T11:14:00Z

sdk/go/common/workspace/project.json

-            "required":[
-                "type"
-            ],
-            "if":{


Are we treating type: array without items as array then?

I've removed because it trips up the validation when only defining default property. It says "if-then failed because there is no items property" but I didn't even specify what the type is.

I've added validation when loading the project to achieve the same result.

Frassle · 2022-10-30T11:14:52Z

sdk/go/common/workspace/project.json

@@ -324,7 +312,8 @@
                "secret":{
                    "type":"boolean"
                },
-                "default":{ }
+                "default":{ },
+                "value": { }


I think we should split configTypeDeclaration into configTypeDeclaration without value and configValueDeclaration with just value

I tried this and refactored the code but the JSON validation errors are a lot more verbose than the errors returned by the current logic. So I reverted it back to this.

Frassle · 2022-10-30T11:17:14Z

sdk/go/common/workspace/project_test.go

@@ -434,6 +456,122 @@ config:
 	assert.Equal(t, "us-west-1", getConfigValue(t, stack.Config, "aws:region"))
 }

+func TestUntypedProjectConfigValuesAreNotValidated(t *testing.T) {


Can we have another test very much like this one but checking that "Undeclared" values are still ok. So have instanceSize in the stackYaml but don't declare it at all in the projectYaml

Config values in a stack don't need a declaration in the project when:

They are non-project namespaced values (i.e. aws:<something>)

The project does not declare any config yet (for backward-compatibility: how things are used right now)

When the project does declare any config value and the stack uses an underclared value, we show an error saying that the value should be declared. Do we want to get rid of point 2?

Dropped 2. and now we are more chill about having stacks declare config values not in the project (see tests)

Yeh speaking to Luke about this we think we should err on being lenient here for now, something to revisit in the future once there's a lot of uptake on this.

Frassle · 2022-10-30T11:18:44Z

sdk/go/common/workspace/project.go

 	}
 }

+func isArray(value interface{}) bool {
+	_, ok := value.([]interface{})


Is this safe? What if value is a []int?

It is safe when checking against unmarshalled values from JSON/YAML which is where this is applicable

Frassle · 2022-10-30T11:19:47Z

sdk/go/common/workspace/project.go

@@ -104,13 +104,29 @@ type ProjectConfigItemsType struct {
 }

 type ProjectConfigType struct {


Wonder if its worth splitting this type DU style and writing custom marshallers. Maybe some thing to play with when we're not under time pressure for the release this week.

I suggest we keep this in mind for later, it would require a refactor everywhere but it would make the code somewhat nicer when we model the fact that value is a special thing

Frassle

Looks like this gets us to a good starting place for this feature. A few things to revisit but good for the release this week.

…validation

Zaid-Ajaj · 2022-10-31T01:13:10Z

bors merge

bors · 2022-10-31T01:48:31Z

Build succeeded:

bors-ok

treykasada · 2023-01-24T08:17:56Z

Hey @Zaid-Ajaj! 👋

Apologies for the random ping, but could you give some context on why defining project-level schemas for non-project-namespaced configuration items (e.g. aws:region) is invalid?

I just ran into that particular validation check while attempting to define a project-level schema that says "stacks must all specify an aws:region", and was very confused.

My understanding is that we're allowed to use arbitrary namespaces when defining stack configuration items, and that the <project name> namespace is just a default (in fact I usually prefer to avoid that namespace because I'm not a fan of duplicating the project name all over the place 😅). But this validation rule seems to imply we should only be using the <project name> namespace for our configuration? Is my mental model for this stuff just incorrect?

Frassle · 2023-01-24T08:28:23Z

Apologies for the random ping, but could you give some context on why defining project-level schemas for non-project-namespaced configuration items (e.g. aws:region) is invalid?

Your project doesn't define those properties, the provider binaries themselves do. That is it's not up to you to say that "aws:region" is a string.

Short term this isn't ideal, because we're not currently pulling that information from providers so non-project config doesn't get type checked, but this is no worse than before where nothing was type checked.
What we didn't want to happen was for users to start filling in types for these properties, and then those type declarations clashing with the ones from providers once we added that support.

I just ran into that particular validation check while attempting to define a project-level schema that says "stacks must all specify an aws:region", and was very confused.

That's an interesting use case that we hadn't considered. Forcing an optional provider field to be required, I'll think about if we can support that more directly.

My understanding is that we're allowed to use arbitrary namespaces when defining stack configuration items...
But this validation rule seems to imply we should only be using the namespace for our configuration?

Yes this is a limitation of project level config currently. It only works for the namespace because we don't know if other namespaces are provider namespaces or users extra namespaces. Again this will probably get changed somehow once we work out more details about how provider schemas will supported.

treykasada · 2023-01-24T08:55:08Z

@Frassle Ahhh, okay. Yep, that all makes sense to me. Thanks so much for elaborating (and for such a quick response)! 😄

Frassle · 2023-01-24T09:41:55Z

I've raised #11965 to track your request of marking provider properties as required.

Zaid-Ajaj requested review from Frassle and iwahbe October 30, 2022 00:13

Frassle reviewed Oct 30, 2022

View reviewed changes

Frassle approved these changes Oct 30, 2022

View reviewed changes

typing made optional in hierarchical config and relaxed stack config …

88558b2

…validation

Zaid-Ajaj force-pushed the zaid/hierarchical-config-second-pass branch from 944bca2 to 88558b2 Compare October 30, 2022 22:42

bors bot merged commit a0271f7 into master Oct 31, 2022

bors bot deleted the zaid/hierarchical-config-second-pass branch October 31, 2022 01:48

Frassle mentioned this pull request Jan 24, 2023

Allow project config to mark provider config as required #11965

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hierarchical config: optional typing extended short-hand syntax #11192

Hierarchical config: optional typing extended short-hand syntax #11192

Zaid-Ajaj commented Oct 30, 2022

pulumi-bot commented Oct 30, 2022 •

edited

Frassle Oct 30, 2022

Zaid-Ajaj Oct 30, 2022

Frassle Oct 30, 2022

Zaid-Ajaj Oct 30, 2022

Frassle Oct 30, 2022

Zaid-Ajaj Oct 30, 2022 •

edited

Zaid-Ajaj Oct 30, 2022

Frassle Oct 30, 2022

Frassle Oct 30, 2022

Zaid-Ajaj Oct 30, 2022

Frassle Oct 30, 2022

Zaid-Ajaj Oct 30, 2022

Frassle left a comment

Zaid-Ajaj commented Oct 31, 2022

bors bot commented Oct 31, 2022

treykasada commented Jan 24, 2023

Frassle commented Jan 24, 2023

treykasada commented Jan 24, 2023

Frassle commented Jan 24, 2023

		@@ -104,13 +104,29 @@ type ProjectConfigItemsType struct {
		}

		type ProjectConfigType struct {

Hierarchical config: optional typing extended short-hand syntax #11192

Hierarchical config: optional typing extended short-hand syntax #11192

Conversation

Zaid-Ajaj commented Oct 30, 2022

Description

Checklist

pulumi-bot commented Oct 30, 2022 • edited

Changelog

[uncommitted] (2022-10-31)

Features

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zaid-Ajaj Oct 30, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Frassle left a comment

Choose a reason for hiding this comment

Zaid-Ajaj commented Oct 31, 2022

bors bot commented Oct 31, 2022

treykasada commented Jan 24, 2023

Frassle commented Jan 24, 2023

treykasada commented Jan 24, 2023

Frassle commented Jan 24, 2023

pulumi-bot commented Oct 30, 2022 •

edited

Zaid-Ajaj Oct 30, 2022 •

edited