Skip to content

Commit

Permalink
Properly error out when input file doesn't exist (#1640)
Browse files Browse the repository at this point in the history
<!--
Before you open the request please review the following guidelines and
tips to help it be more easily integrated:

 - Describe the scope of your change - i.e. what the change does.
 - Describe any known limitations with your change.
- Please run any tests or examples that can exercise your modified code.

Thank you for contributing! We will try to test and integrate the change
as soon as we can, but be aware we have many GitHub repositories to
manage and can't immediately respond to every request. There is no need
to bump or check in on a pull request (it will clutter the discussion of
the request).

Also don't be worried if the request is closed or not integrated
sometimes the priorities of Bitnami might not match the priorities of
the pull request. Don't fret, the open source community thrives on forks
and GitHub makes it easy to keep your changes in a forked repo.
 -->

**Description of the change**

<!-- Describe the scope of your change - i.e. what the change does. -->

This fixes a bug in the `kubeseal` CLI where it silently and
errorneously does nothing and returns a 0 (eg: success) exit code when a
specified `--secret-file` does not exist (or is unreadable).

```bash
$ kubeseal \
    --kubeconfig ${KUBECONFIG:-~/.kube/config} \
    --controller-namespace=sealed-secrets \
    --controller-name=sealed-secrets-controller \
    --secret-file doesntexist.yaml; echo $?
0
```

It looks like there was a simple oversight in #1030, and as noted in
that PR:

> As with all refactors, bugs could be introduced.

I just tweaked the exit logic to output an error message and exit with
nonzero status, which I think is much more reasonable behavior as it is
both less surprising and also follows what other POSIX-y CLIs do when
they are pointed to files that don't exist.

**Benefits**

<!-- What benefits will be realized by the code change? -->

Instead of silently doing nothing and erroneously returning a success
status/exit code, this will now clearly indicate that the problem
occurred and programmatically indicate a non-successful result. I'm not
super happy with the error message (I think it could be written better),
but this is my first PR for a Go project, so I'm not sure what the
culture of error messages is in the Go community (constructive guidance
will be welcome and appreciated!).

**Possible drawbacks**

<!-- Describe any known limitations with your change -->

Anybody who has been relying on the previous -- non-standard -- behavior
will be impacted by this change. But, also as noted in #1030:

> **The currently exposed kubeseal interface must be considered
unstable, it will change going forward.**

**Applicable issues**

<!-- Enter any applicable Issues here (You can reference an issue using
#) -->
- fixes #

**Additional information**

<!-- If there's anything else that's important and relevant to your pull
request, mention that information here.-->

This is my first ever foray into Golang, so I might need more
hand-holding than your average contributor, especially around how to
test this. TIA. 😄

Edit: Added example.

Signed-off-by: blag <[email protected]>
  • Loading branch information
blag authored Dec 5, 2024
1 parent c2aef0d commit 46e1372
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion cmd/kubeseal/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func runCLI(w io.Writer, cfg *config) (err error) {
// #nosec G304 -- should open user provided file
f, err := os.Open(flags.inputFileName)
if err != nil {
return nil
return fmt.Errorf("Could not read file specified with --secret-file")
}
// #nosec: G307 -- this deferred close is fine because it is not on a writable file
defer f.Close()
Expand Down

0 comments on commit 46e1372

Please sign in to comment.