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

Validate Name, Version and Enviroment #7896

Merged
merged 16 commits into from Sep 17, 2021

Conversation

iwahbe
Copy link
Member

@iwahbe iwahbe commented Sep 2, 2021

Description

Adds validators for Package.Name, corrects the validator for Package.Version, and adds a validator for Default.Environment

Fixes #7893

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • Yes, there are changes in this PR that warrants bumping the Pulumi Service API version

For the full path:
Package.Name
Package.Version
Package.Property.Default
@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-azuread with merge commit f6d8dbd

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-random with merge commit f6d8dbd

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-kubernetes with merge commit f6d8dbd

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-gcp with merge commit f6d8dbd

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-aws with merge commit f6d8dbd

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-azure with merge commit f6d8dbd

@iwahbe
Copy link
Member Author

iwahbe commented Sep 2, 2021

/run-acceptance-tests

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Please view the results of the PR Build + Acceptance Tests Run Here

@iwahbe iwahbe self-assigned this Sep 2, 2021
@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-random with merge commit c1f8446

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-azuread with merge commit c1f8446

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-kubernetes with merge commit c1f8446

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-gcp with merge commit c1f8446

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-random with merge commit 94aa4e9

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-azuread with merge commit 94aa4e9

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-kubernetes with merge commit 94aa4e9

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-gcp with merge commit 94aa4e9

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-random with merge commit 40cbeeb

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-azuread with merge commit 40cbeeb

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-kubernetes with merge commit 40cbeeb

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-gcp with merge commit 40cbeeb

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-aws with merge commit 40cbeeb

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-azure with merge commit 40cbeeb

@github-actions
Copy link

github-actions bot commented Sep 2, 2021

Diff for pulumi-random with merge commit 1315cb7

@github-actions
Copy link

Diff for pulumi-azuread with merge commit ef0d228

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit ef0d228

@t0yv0 t0yv0 self-requested a review September 14, 2021 17:41
@github-actions
Copy link

Diff for pulumi-aws with merge commit ef0d228

@github-actions
Copy link

Diff for pulumi-gcp with merge commit ef0d228

@github-actions
Copy link

Diff for pulumi-azure with merge commit ef0d228

Comment on lines 75 to 76
if err == nil && len(diags) != 0 {
return errors.New("schema validation failed")
Copy link
Member

@pgavlin pgavlin Sep 15, 2021

Choose a reason for hiding this comment

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

Suggested change
if err == nil && len(diags) != 0 {
return errors.New("schema validation failed")
if err == nil && diags.HasErrors() {
return errors.New("schema validation failed")

Copy link
Member

Choose a reason for hiding this comment

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

Looks like this change didn't get applied?

@t0yv0
Copy link
Member

t0yv0 commented Sep 15, 2021

Since Pat is already here taking a raincheck and I'm a bit underwater with reviews, will review tomorrow..

@github-actions
Copy link

Diff for pulumi-azuread with merge commit 73f4b19

@github-actions
Copy link

Diff for pulumi-random with merge commit 73f4b19

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit 73f4b19

@github-actions
Copy link

Diff for pulumi-aws with merge commit 73f4b19

@github-actions
Copy link

Diff for pulumi-gcp with merge commit 73f4b19

@github-actions
Copy link

Diff for pulumi-azure with merge commit 73f4b19

@iwahbe iwahbe requested a review from pgavlin September 15, 2021 21:17
@t0yv0
Copy link
Member

t0yv0 commented Sep 17, 2021

LGTM modulo Pat's comment, should the clause short-circuit on len(diags) = 0 or just errors? assuming there are warnings and errors. We should not stop if just warnings?

Also. Related thought.

This uses JSON schema meta-schema:

#7952

Potentially the same could be accomplished by adding validation patterns http://json-schema.org/understanding-json-schema/reference/regular_expressions.html#example and riding on generic JSON schema validation checks.

Don't wait though till that lands, check this in.

@github-actions
Copy link

Diff for pulumi-random with merge commit 0cd63e7

@github-actions
Copy link

Diff for pulumi-azuread with merge commit 0cd63e7

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit 0cd63e7

Copy link
Member

@pgavlin pgavlin left a comment

Choose a reason for hiding this comment

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

LGTM!

@github-actions
Copy link

Diff for pulumi-aws with merge commit 0cd63e7

@github-actions
Copy link

Diff for pulumi-gcp with merge commit 0cd63e7

@github-actions
Copy link

Diff for pulumi-azure with merge commit 0cd63e7

@iwahbe iwahbe merged commit 7f90e12 into master Sep 17, 2021
@pulumi-bot pulumi-bot deleted the iwahbe/7893/package-bind-validation branch September 17, 2021 19:12
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.

pulumi schema check does not fail on empty name or empty version
3 participants