-
Notifications
You must be signed in to change notification settings - Fork 84
[package-spec] - Added support for '*.csv' and '*.gz' file patterns in terraform deployer spec #926
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
Conversation
spec/changelog.yml
Outdated
- description: Added support for '*.csv' and '*.gz' file patterns in terraform deployer spec. | ||
type: enhancement | ||
link: https://github.com/elastic/package-spec/pull/926 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changelog entry can be moved to 3.4.1-next
version section.
- description: Csv files to add into terraform resources | ||
type: file | ||
pattern: '^.*\.csv$' | ||
required: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could it be added text/plain
as content media type ?
- description: Csv files to add into terraform resources | |
type: file | |
pattern: '^.*\.csv$' | |
required: false | |
- description: Csv files to add into terraform resources | |
type: file | |
contentMediaType: "text/plain" | |
pattern: '^.*\.csv$' | |
required: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mime type for csv is text/csv, would that suit better here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right that would suit better.
Currently, text/csv
mime type is not supported in package-spec:
package-spec/code/go/internal/validator/content.go
Lines 16 to 34 in a50d2bc
func validateContentType(fsys fs.FS, path string, contentType spectypes.ContentType) error { | |
switch contentType.MediaType { | |
case "application/x-yaml": | |
v, _ := contentType.Params["require-document-dashes"] | |
requireDashes := (v == "true") | |
if requireDashes { | |
err := validateYAMLDashes(fsys, path) | |
if err != nil { | |
return err | |
} | |
} | |
case "application/json": | |
case "text/markdown": | |
case "text/plain": | |
default: | |
return fmt.Errorf("unsupported media type (%s)", contentType) | |
} | |
return nil | |
} |
One option is to add another case for text/csv
into that function (as it is text/markdown
or text/plain
) for instance. And I guess it is not needed to add any specific validation for that media type.
@mrodm, made the suggested changes |
💚 Build Succeeded
History
cc @ShourieG |
@ShourieG another related PR elastic/integrations#14524. |
@mrodm, I'm not getting the option to merge, can you help me to get it merged. |
Sure, I'll merge it 👍 |
What does this PR do?
Adds support for '.csv' and '.gz' file patterns in terraform deployer spec.
Why is it important?
Unblocks integration PRs which utilise such patterns.
Checklist
test/packages
that prove my change is effective.spec/changelog.yml
.Related issues
Alerts v2
data stream integrations#14443