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

List secrets with last updated, input bugfix, create/update instructions update #177

Merged
10 commits merged into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changes/unreleased/Bugfix-20230925-130703.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kind: Bugfix
body: fix bug not reading files when using -f
time: 2023-09-25T13:07:03.642418-04:00
3 changes: 3 additions & 0 deletions .changes/unreleased/Feature-20230925-130715.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kind: Feature
body: show last updated when listing secrets
time: 2023-09-25T13:07:15.386994-04:00
6 changes: 4 additions & 2 deletions src/cmd/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@ import (
var dataFile string

func readInputConfig() {
viper.SetConfigType("yaml")
Copy link
Author

Choose a reason for hiding this comment

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

IIRC -f filename.yaml wasn't working until I moved this up here from outside of the case "-" block.

Copy link
Author

Choose a reason for hiding this comment

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

can't think of a reason for this to not be applied in certain conditions vs others, so it makes sense to move to the top.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is explicitly set to yaml below since viper has no way to infer the filetype from the stdin input.

viper.SetConfigFile() defines the path, name and extension of the config file. Then viper.ReadInConfig() will know how to read it later.

If somebody is using a .json file I would assume viper would internally reset the configType below?

In any case, @rocktavious has mentioned that we should look into a better solution for reading user input in place of viper

Copy link
Collaborator

Choose a reason for hiding this comment

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

ya we need to fix this reading from input problems globally

switch dataFile {
case ".":
// TODO: does this block ever actually ever run?
Copy link
Contributor

Choose a reason for hiding this comment

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

Somebody could be using this case.
Setting -f to . is called out in the command help docs for opslevel create --help (create.go) and opslevel update --help (update.go).

Copy link
Author

Choose a reason for hiding this comment

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

I could not find a way to get it to trigger though, at least on bash/zsh. IIRC I ran into this problem before, passing . as an argument in CLI's is a bad idea 😅

I don't think we should remove it immediately, but we should look into it because if this branch never gets called the data.yaml stuff is confusing.

viper.SetConfigFile("./data.yaml")
case "-":
if isStdInFromTerminal() {
log.Info().Msg("Reading input directly from command line...")
// TODO: this can take up to half a second to output which interrupts the user's experience
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 If this is working correctly users shouldn't see this unless they are manually typing in yaml formatted input. If that's not the case lmk and I can take a closer look

Copy link
Author

Choose a reason for hiding this comment

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

To clarify, this happens to me only when manually typing in the command. (The check isStdInFromTerminal() is fine)

What happens is: I start typing, then a second later the message pops up, I lose track of what I've typed, then CTRL+C and re-run the command and start typing again. It's a UX annoyance

log.Info().Msg("Reading input directly from command line... Press CTRL+D to stop typing")
This conversation was marked as resolved.
Show resolved Hide resolved
}
viper.SetConfigType("yaml")
viper.ReadConfig(os.Stdin)
default:
viper.SetConfigFile(dataFile)
Expand Down
44 changes: 18 additions & 26 deletions src/cmd/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,15 @@ package cmd
import (
"encoding/json"
"fmt"
"io"
"os"

"github.com/creasty/defaults"

"github.com/opslevel/opslevel-go/v2023"
"gopkg.in/yaml.v3"

"github.com/opslevel/cli/common"

"github.com/spf13/cobra"
"github.com/spf13/viper"
)

var secretAlias string
Expand All @@ -24,7 +22,8 @@ var createSecretCmd = &cobra.Command{
Long: `Create a team-owned secret`,
Example: `
cat << EOF | opslevel create secret --alias=my-secret-alias -f -
owner: "devs"
owner:
alias: "devs"
value: "my-really-secure-secret-shhhh"
EOF`,
Run: func(cmd *cobra.Command, args []string) {
Expand Down Expand Up @@ -70,9 +69,9 @@ var listSecretsCmd = &cobra.Command{
if isJsonOutput() {
common.JsonPrint(json.MarshalIndent(list, "", " "))
} else {
w := common.NewTabWriter("ALIAS", "ID", "OWNER")
w := common.NewTabWriter("ALIAS", "ID", "OWNER", "UPDATED_AT")
for _, item := range list {
fmt.Fprintf(w, "%s\t%s\t%s\t\n", item.Alias, item.ID, item.Owner.Alias)
fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", item.Alias, item.ID, item.Owner.Alias, item.Timestamps.UpdatedAt.Time)
}
w.Flush()
}
Expand All @@ -84,16 +83,17 @@ var updateSecretCmd = &cobra.Command{
Short: "Update an OpsLevel secret",
Long: `Update an OpsLevel secret`,
Example: `
cat << EOF | opslevel update secret XXX_secret_id_XXX -f -
owner: "platform"
value: "09sdf09werlkewlkjs0-9sdf
EOF
`,
cat << EOF | opslevel update secret XXX_secret_id_XXX -f -
owner:
alias: "platform"
value: "09sdf09werlkewlkjs0-9sdf
EOF`,
Args: cobra.ExactArgs(1),
ArgAliases: []string{"ID"},
Run: func(cmd *cobra.Command, args []string) {
secretId := args[0]
input, err := readSecretInput()
cobra.CheckErr(err)
secret, err := getClientGQL().UpdateSecret(secretId, *input)
cobra.CheckErr(err)
fmt.Println(secret.ID)
Expand All @@ -102,8 +102,8 @@ var updateSecretCmd = &cobra.Command{

var deleteSecretCmd = &cobra.Command{
Use: "secret ID|ALIAS",
Short: "Delete a system",
Long: `Delete a system from OpsLevel`,
Short: "Delete a secret",
Long: `Delete a secret from OpsLevel`,
Args: cobra.ExactArgs(1),
ArgAliases: []string{"ID", "ALIAS"},
Run: func(cmd *cobra.Command, args []string) {
Expand All @@ -115,21 +115,13 @@ var deleteSecretCmd = &cobra.Command{
}

func readSecretInput() (*opslevel.SecretInput, error) {
file, err := io.ReadAll(os.Stdin)
cobra.CheckErr(err)
var evt struct {
Owner string `yaml:"owner"`
Value string `yaml:"value"`
}
cobra.CheckErr(yaml.Unmarshal(file, &evt))
secretInput := &opslevel.SecretInput{}
if err := defaults.Set(secretInput); err != nil {
readInputConfig()
evt := &opslevel.SecretInput{}
viper.Unmarshal(&evt)
if err := defaults.Set(evt); err != nil {
This conversation was marked as resolved.
Show resolved Hide resolved
return nil, err
}

secretInput.Value = evt.Value
secretInput.Owner = *opslevel.NewIdentifier(evt.Owner)
return secretInput, nil
return evt, nil
}

func init() {
Expand Down
Loading