redpanda-data / redpanda

Redpanda is a streaming data platform for developers. Kafka API compatible. 10x faster. No ZooKeeper. No JVM!
https://redpanda.com
9.42k stars 577 forks source link

rpk profile edit eats errors #18698

Open supahcraig opened 3 months ago

supahcraig commented 3 months ago

Version & Environment

Redpanda version: (use rpk version): rpk version v24.1.3 (rev 1cae54a38e)

although I have seen this on prior versions.

What went wrong?

When editing an rpk profile using rpk profile edit, if there are errors in your yaml it will report that your profile was updated successfully, when in fact it will silently revert to the prior config. It isn't only for basic yaml formatting errors, but also if the new yaml isn't a valid redpanda config. That is, if you use the wrong parameter name (i.e. "brokers" where "addresses" is expected).

Side Note: inconsistency here is not fun. Some places use brokers, some use addresses, some use a bracketed array with quotes around the url, and still others allow for the port to be on a separate line from the address. Makes it hard to just "know" how to build/troubleshoot a config w/o consulting the docs.

What should have happened instead?

It needs to report back that there was an issue (ideally with the actual context of the problem, error stack etc) to alert you that your config changes have not been applied. It definitely should not tell you that your profile has been updated when in fact it has not.

In the event you've made a significant number of changes to the config, having it revert to the prior config is especially frustrating. I think it would be preferable to actually save the changes but report that the new config is invalid (with errors, line #'s etc). This will save the user the pain of needing to re-do all of those changes, especially since they currently don't have the benefit of even knowing what went wrong (or that it even went wrong). Allowing the invalid config to take effect has the positive side effect of NOT allowing the user to unknowingly use the prior config after making changes that didn't take, since conceivably the new config would make subsequent rpk commands fail.

How to reproduce the issue?

  1. show existing profile: rpk profile print note there is nothing specific about this profile, it just happens to be the one I am currently using
name: ec2_TLStf
description: EC2 1 node cluster using TF-generated certs
prompt: hi-red, "[%n]"
from_cloud: false
kafka_api:
    brokers:
        - 3.22.217.54:9092
    tls:
        insecure_skip_verify: true
admin_api:
    addresses:
        - 3.22.217.54:9644
    tls:
        insecure_skip_verify: true
schema_registry: {}
  1. Edit the profile in a way that makes it invalid: rpk profile edit in my case I was trying to determine if you could disable TLS by guessing at the setting
name: ec2_TLStf
description: EC2 1 node cluster using TF-generated certs
prompt: hi-red, "[%n]"
from_cloud: false
kafka_api:
    brokers:
        - 3.22.217.54:9092
    tls:
        enabled: false
        insecure_skip_verify: true
admin_api:
    addresses:
        - 3.22.217.54:9644
    tls:
        insecure_skip_verify: true
schema_registry: {}
  1. Save & exit from the editor, shows this output:
Profile "ec2_TLStf" updated successfully.
  1. Show the "newly updated" profile: rpk profile print

note that this "new" profile is exactly as it was before the changes were made.

name: ec2_TLStf
description: EC2 1 node cluster using TF-generated certs
prompt: hi-red, "[%n]"
from_cloud: false
kafka_api:
    brokers:
        - 3.22.217.54:9092
    tls:
        insecure_skip_verify: true
admin_api:
    addresses:
        - 3.22.217.54:9644
    tls:
        insecure_skip_verify: true
schema_registry: {}

JIRA Link: CORE-3109

github-actions[bot] commented 2 weeks ago

This issue hasn't seen activity in 3 months. If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in two weeks.