Skip to content

cmd: add two new command line programs. #136

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

Merged
merged 14 commits into from
May 15, 2025
Merged

cmd: add two new command line programs. #136

merged 14 commits into from
May 15, 2025

Conversation

pwhelan
Copy link
Contributor

@pwhelan pwhelan commented May 5, 2025

cmd: add new command line programs; fluentbit-convert and fluentbit-validate.

This PR adds two a new command line utilities backed by go-fluentbit-config:

  • fluentbit-validate: validates the format of a fluentbit configuration file.
  • fluentbit-convert: converts configurations from and to json, classic and yaml.
  • fluentbit-config: converts and or validates a fluentbit configuration, even against a specific schema.

@pwhelan pwhelan requested a review from a team May 5, 2025 16:19
@pwhelan pwhelan requested a review from niedbalski May 7, 2025 17:37
pwhelan added 5 commits May 14, 2025 16:49
…alidate.

This commit adds two new command line utilities backed by go-fluentbit-config:

  * fluentbit-validate: validates the format of a fluentbit configuration file.
  * fluentbit-convert: converts configurations from and to json, classic and yaml.

Signed-off-by: Phillip Adair Stewart Whelan <[email protected]>
Signed-off-by: Phillip Adair Stewart Whelan <[email protected]>
Signed-off-by: Phillip Adair Stewart Whelan <[email protected]>
@pwhelan pwhelan enabled auto-merge May 15, 2025 00:16
pwhelan added 9 commits May 15, 2025 10:38
Signed-off-by: Phillip Adair Stewart Whelan <[email protected]>
Signed-off-by: Phillip Adair Stewart Whelan <[email protected]>
Signed-off-by: Phillip Adair Stewart Whelan <[email protected]>
Signed-off-by: Phillip Adair Stewart Whelan <[email protected]>
Signed-off-by: Phillip Adair Stewart Whelan <[email protected]>
@pwhelan pwhelan merged commit bd06dfa into main May 15, 2025
1 check passed
@pwhelan pwhelan deleted the pwhelan-cmd branch May 15, 2025 15:22
Copy link
Contributor

@prashantv prashantv left a comment

Choose a reason for hiding this comment

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

nice, LGTM

Comment on lines +291 to +292
/fluentbit-convert
/fluentbit-validate
Copy link
Contributor

Choose a reason for hiding this comment

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

need to update this to the new fluentbit-config binary

os.Exit(1)
}

ext := filepath.Ext(args[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

very tiny nit, generally in go, recommend declaring variables close to use, so I'd move this to the line before getFormatFromExt

Comment on lines +45 to +46
fmt.Printf("ERROR: %s\n", err)
os.Exit(1)
Copy link
Contributor

Choose a reason for hiding this comment

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

for future, since we have a lot of this duplicated, it's worth DRYing this up by using a pattern like:

func main() {
  if err := run(); err != nil {
    fmt.Printf("ERROR: %s\n", err)
    os.Exit(1)
  }
}

and then putting all the logic into a run() error function so we can do if err != nil { return err }

schema, err = fluent.GetSchema(schemaVersion)
if err != nil {
fmt.Printf("ERROR: %s\n", err)
os.Exit(2)
Copy link
Contributor

Choose a reason for hiding this comment

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

curious why Exit(2) here, but Exit(1) for other cases (probably worth a comment in the code too)

"output format, one of: json, yaml (yml), ini or conf")
pflag.StringVarP(&outputFilename, "output", "o", "",
"output file (optional, default is stdout)")
pflag.BoolVarP(&checkSchema, "schema", "s", false,
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems like something we should enable by default, so maybe we flip it to no-schema-check or something?

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.

3 participants