Skip to content

SD-1615 add secret names to the SD cli output#1643

Merged
Brian Strauch (brianstrauch) merged 10 commits intomainfrom
xx-add-secret-names
Jan 11, 2023
Merged

SD-1615 add secret names to the SD cli output#1643
Brian Strauch (brianstrauch) merged 10 commits intomainfrom
xx-add-secret-names

Conversation

@xiangxin72
Copy link
Contributor

Checklist

  1. [CRUCIAL] Is the change for CP or CCloud functionalities that are already live in prod?
    • yes: ok (but behind a feature flag)

What

SD CLI should also print out the secret names stored by the backend, which is already part of the api response

References

https://confluentinc.atlassian.net/wiki/spaces/DI/pages/2920416089/Design+Note+Stream+Designer+API+and+CLI+with+source+code+and+secrets

Test & Review

Comment on lines 129 to 131
if len(name) > secretNameSizeLimit {
return nil, fmt.Errorf(`secret name "%s" cannot exceed %d characters`, name, secretNameSizeLimit)
}

Choose a reason for hiding this comment

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

Is this validation not performed by the backend?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is validated at the backend, but I thought doing validation here would be a better user exp. Or does CLI not checking such in general to avoid duplicate implementation?

Choose a reason for hiding this comment

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

Yeah, I'd rather not run into a situation where the secret size limit gets changed in the backend but not here (since we don't have any system tests for this)

Choose a reason for hiding this comment

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

If the backend is going to return an error message that says the same thing anyway, there should be no reason to write the same code twice!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's fair, I've removed the secret name check, but kept the regex check in place, because that's far more likely to be violated hence better to have a local client side validation.

@brianstrauch Brian Strauch (brianstrauch) deleted the xx-add-secret-names branch January 11, 2023 21:51
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