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

nsc generate nkey outputs good response to stderr rather than stdout #580

Open
kimjarvis opened this issue Apr 9, 2023 · 4 comments · May be fixed by #590
Open

nsc generate nkey outputs good response to stderr rather than stdout #580

kimjarvis opened this issue Apr 9, 2023 · 4 comments · May be fixed by #590

Comments

@kimjarvis
Copy link

nsc generate nkey outputs a good response to stderr, rather than stdout, which is unexpected.

root@instance:/# nsc generate nkey -a --store 2> e
root@instance:/# nsc generate nkey -a --store
AALS5NG2EJCJGCBTCRRH4PJO2R2UFHVAAIPLZEI5OPHHGW5T67DFNJRY
account key stored /root/.local/share/nats/nsc/keys/keys/A/AL/AALS5NG2EJCJGCBTCRRH4PJO2R2UFHVAAIPLZEI5OPHHGW5T67DFNJRY.nk

In particular, I would expect the following ansible to work. But, to make it work i had to change stdout to stderr.

    - name: Generate account signing key
      ansible.builtin.shell: >-
        nsc generate nkey -a --store
        {{ client_config }}
      register: result

    - name: Get the client account signing key 
      ansible.builtin.set_fact: client_account_signing_key="{{result.stdout_lines[0]}}"
@kimjarvis kimjarvis changed the title nsc generate nkey outputs good response to stderr rather than stdout nsc generate nkey outputs good response to stderr rather than stdout Apr 10, 2023
@aricart
Copy link
Member

aricart commented Apr 18, 2023

Verified, the generated key should be sent to stdout. Now wondering if generate key and list keys should output JSON - I would consider the informational message (account key stored...) to be stderr.

@kimjarvis
Copy link
Author

kimjarvis commented Apr 23, 2023

Maybe, generate key could have an option for json output like this:

nsc generate nkey -a --store --json
{
	"type": "account",
	"key": "ACWGQFI6UZMVZREABW4GZVXQA6XTJBGGKZJLJHBPEDQLWMOJQLM3IVRB",
	"path": "/root/.local/share/nats/nsc/keys/keys/A/CW/ACWGQFI6UZMVZREABW4GZVXQA6XTJBGGKZJLJHBPEDQLWMOJQLM3IVRB.nk"
}

It would be really helpful, for automation, for there to be an option to output json on every nsc command. But, that is not the subject of this ticket.

All nsc commands, that I looked at today, output good responses to stderr, rather than stdout, which is unexpected. All of the following commands output to stderr with nothing at all going to stdout. In my script, all of them complete successfully and return rc 0.

nsc edit operator --sk OAKKUPVDAEOXMRWTG7MKOAFJNX2I2LHJQMLCXKHDLNQB4OZHGMRX7XWU
nsc describe operator --raw --output-file $HOME/OT.jwt      --all-dirs $HOME/O
nsc add operator -u $HOME/OT.jwt --force      --all-dirs $HOME/V
nsc add account -n T         --all-dirs $HOME/O
nsc edit account -n T --sk ABZRCHNMDORQW5H4WU6HQLQSJZQVVG72L77DRT64RB4V3JWNESS7LCIK
nsc describe account -n T --raw --output-file $HOME/AT.jwt      --all-dirs $HOME/O 
nsc import account --file $HOME/AT.jwt --overwrite      --all-dirs $HOME/V
nsc add user  --account T  --name easy   --all-dirs $HOME/V --private-key  ABZRCHNMDORQW5H4WU6HQLQSJZQVVG72L77DRT64RB4V3JWNESS7LCIK
nsc generate creds -a T -n easy -o $HOME/Ueasy.creds      --all-dirs $HOME/V 

I would have expected that all of these successful commands would have written all of their output to stdout instead. In general, modern commands that are targeted towards automation, such as kubectl, write almost everything to stdout, to make automation simpler.

@aricart
Copy link
Member

aricart commented May 3, 2023

All the above commands (with the exception of nsc generate nkey) return a report - the report was always directed to stderr (cobra is doing this). I have changed it to output to stdout - but it is possible that the report will have some error line [ERR] and at the moment it is impossible to separate those. If the report has an actual error that will be rendered to stderr (along with the report to stdout)

Do note that for the describe commands do support --json, so you can always do:
export C1_PK=$(nsc describe account C1 --json | jq -r .sub) to read public ids etc.

@aricart
Copy link
Member

aricart commented May 3, 2023

The main issue here is that the change of the PR would indeed break havoc on current scripts - so this may be a no-go until we have a nondisruptive way of changing this as an opt-in/out option or a major version bump for nsc.

aricart added a commit that referenced this issue May 3, 2023
…ented editing due to JetStream default values in the flag - added a flag that sets the values for the jetstream setting (--js-disable) which sets the values to 0

Fixes #580
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 a pull request may close this issue.

2 participants