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

yaml/v2: Detect file patterns automatically #2871

Closed
Tracked by #1971
EronWright opened this issue Mar 11, 2024 · 0 comments · Fixed by #2873
Closed
Tracked by #1971

yaml/v2: Detect file patterns automatically #2871

EronWright opened this issue Mar 11, 2024 · 0 comments · Fixed by #2873
Assignees
Labels
area/yaml kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@EronWright
Copy link
Contributor

EronWright commented Mar 11, 2024

There's an asymmetrical experience in ConfigGroup vs ConfigFile when it comes to the handling of non-existent files. ConfigFile produces an error, whereas ConfigGroup doesn't because the latter interprets all paths as patterns. For example, consider this program:

resources:
  cf:
    type: kubernetes:yaml/v2:ConfigFile
    properties:
      file: ./manifest.yaml
  cg:
    type: kubernetes:yaml/v2:ConfigGroup
    properties:
      files:
        - ./manifest.yaml

I feel that the behavior of the two resources should be consistent when manifest.yaml is non-existent, but in actuality the ConfigGroup would silently produce no children, which the program author probably didn't intend.

My proposal is to detect whether a given file entry is a pattern, by looking for the characters ‘*’, ‘?’, and ‘[’ as is done in Bash filename expansion.

@EronWright EronWright self-assigned this Mar 11, 2024
@EronWright EronWright added kind/bug Some behavior is incorrect or out of spec area/yaml labels Mar 11, 2024
EronWright added a commit that referenced this issue Mar 12, 2024
<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
    for Pulumi's contribution guidelines.

    Help us merge your changes more quickly by adding more details such
    as labels, milestones, and reviewers.-->

### Proposed changes

<!--Give us a brief description of what you've done and what it solves.
-->

This PR teaches `ConfigGroup` to detect whether a given file path is a
glob pattern, to make the handling of non-existent files be consistent
w.r.t `ConfigFile`. In other words, `*.yaml` MAY match a file whereas
`manifest.yaml` MUST match a file.

The detection code looks for special characters '*', '?', and '[' and
respects the escape syntax. Intended to be consistent with:
[https://pkg.go.dev/path/filepath#Match](https://pkg.go.dev/path/filepath#Match)

An alternative to doing pattern detection would be:
1. to expose a `glob` property to toggle globbing, or 
2. a `patterns` property alongside the `files` property for patterns and
non-patterns respectively

### Related issues (optional)

<!--Refer to related PRs or issues: #1234, or 'Fixes #1234' or 'Closes
#1234'.
Or link to full URLs to issues or pull requests in other GitHub
repositories. -->
Closes #2871
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/yaml kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants