Show Server Version in Yorkie CLI

hyun98 commented 1 month ago

What this PR does / why we need it:

Add the functionality to display the version of connected Yorkie server in the version command

Which issue(s) this PR fixes:

Fixes #870

Special notes for your reviewer:

The version information can be viewed by converting it into yaml or json format using the --output, -o flag. This flag accepts json and yaml strings and converts them to the appropriate format for display.
In order to solely check the version of the client CLI without considering the server's version, the --client flag has been included.
The development of this feature was inspired by examining the kubectl code.

Does this PR introduce a user-facing change?:

NONE

Additional documentation:

$ yorkie version -h

Print the version number of Yorkie

Usage:
  yorkie version [flags]

Flags:
      --client          Shows client version only. (no server required)
  -h, --help            help for version
  -o, --output string   One of 'yaml' or 'json'.

Global Flags:
      --rpc-addr string   Address of the rpc server (default "localhost:8080")

Checklist:

[x] Added relevant tests or not required
[ ] Didn't break anything

Summary by CodeRabbit

New Features
- Introduced a new method to retrieve the server version, providing users with version details including Yorkie version, Go version, and build date.
- Enhanced the version command functionality to support output format validation and retrieval of the server version.
- Improved output formatting options for the version command, allowing users to choose between standard, YAML, or JSON outputs.
Bug Fixes
- Improved error handling during server version retrieval, ensuring clearer feedback for users.
Tests
- Added new test cases to validate the functionality of the server version retrieval method.

CLAassistant commented 1 month ago

All committers have signed the CLA.

coderabbitai[bot] commented 1 month ago

Walkthrough

The recent updates enhance the Yorkie CLI by introducing a GetServerVersion feature, enabling users to retrieve and display the server version alongside client version details. This functionality improves usability and transparency, facilitating better debugging and maintenance. The changes span multiple files, including client code, API specifications, and testing frameworks, ensuring seamless integration and robustness.

Changes

Files	Change Summary
`admin/client.go`, `cmd/yorkie/version.go`, `server/rpc/testcases/testcases.go`	Added `GetServerVersion` method to retrieve server version and improved CLI command functionality.
`api/docs/yorkie/v1/admin.openapi.yaml`	Introduced a new API endpoint and schemas for server version retrieval.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant YorkieCLI
    participant AdminService
    participant Server

    User->>YorkieCLI: Request server version
    YorkieCLI->>AdminService: Call GetServerVersion()
    AdminService->>Server: Retrieve version info
    Server-->>AdminService: Return version details
    AdminService-->>YorkieCLI: Send version info
    YorkieCLI-->>User: Display version info

Assessment against linked issues

Objective	Addressed	Explanation
Show server version in Yorkie CLI (870)	✅

🐰 In the meadow, bright and free,
A server's version calls to me.
With each command, a joyful dance,
Yorkie's version takes a chance!
The CLI now sings, a version bright,
For users' ease, it's pure delight! 🌟

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

Tips

### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (invoked as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configuration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.

hyun98 commented 1 month ago

First of all, thank you very much for your review. @hackerwins

D. Questions and suggestions for improvement:

Q) Should we allow users to view the server version without authentication? This could be beneficial for troubleshooting and version compatibility checks.

A) I agree that checking the server version without authentication has several advantages. So, I would like to add a flag that can check the version of a specific addr without authentication. For example, options like --no-auth. Would this be a good option name? I always worry about naming. Additionally, I'm asking just in case, but I understand that you don't mean to remove the authentication itself for version checking, right?

Q) How should we handle different scenarios?

When the server endpoint is incorrect or the server is not running: We should provide a clear error message indicating connection issues.

A) Thank you. If the server version is not loaded properly, I need to modify it to display a more appropriate error message.

When the server is running an older version without the version API: We should gracefully handle this case, perhaps by showing the client version and indicating that the server version is unavailable or outdated.

A) Likewise, we need to show an appropriate message regarding the case you mentioned. For example, Phrases like: 'server version checking is only supported if the server version is 0.4.27 or higher.'

Lastly, there was an error in my code. I tried to use auth, but there was a problem with only loading auth that does not fit the current context because config.Preload was not performed, so I will fix it together.

Thank you :)

hackerwins commented 1 month ago

A) I agree that checking the server version without authentication has several advantages. So, I would like to add a flag that can check the version of a specific addr without authentication. For example, options like --no-auth. Would this be a good option name? I always worry about naming. Additionally, I'm asking just in case, but I understand that you don't mean to remove the authentication itself for version checking, right?

It would be better to remove authentication entirely for checking the server version. Version is often considered public information and it improves the experience for developers who need to quickly check version. I am also curious about whether kubectl's version command checks authentication. 🤔

hyun98 commented 1 month ago

It would be better to remove authentication entirely for checking the server version. Version is often considered public information and it improves the experience for developers who need to quickly check version. I am also curious about whether kubectl's version command checks authentication. 🤔

That's a relief, I asked just in case. Now I understood your intentions correctly. I will try to remove authentication with the version check command. I think removing authentication would be a better experience during the development process.

However, there is one thing that concerns me. I don't know much about security, but I wonder if there will be any security issues due to server version exposure. 🤔

Additionally, kubectl always goes through an authentication process to obtain the server's version information (only when there is no --client option). To obtain version information, the context and auth existing in $HOME/.kube/config are used. This is because the k8s api-server is accessed through information.

krapie commented 1 month ago

When thinking about who will be using this feature, I think the internal developers who combined Yorkie with their system will use this feature to check or troubleshoot something. So I'm not sure about the necessity of exposing this feature to public.

However, there is one thing that concerns me. I don't know much about security, but I wonder if there will be any security issues due to server version exposure. 🤔

@hyun98 I think there can be a malicious attack by penetrating the weakness of the certain server version exposed by this feature.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 51.34%. Comparing base (52d2732) to head (781d639). Report is 1 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #938 +/- ## ========================================== + Coverage 51.31% 51.34% +0.02% ========================================== Files 71 71 Lines 10679 10685 +6 ========================================== + Hits 5480 5486 +6 Misses 4650 4650 Partials 549 549 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hyun98 commented 1 month ago

Thanks for your comment. I agree that the current code lacks the advantage of goroutines. I'll remove it as it increases complexity such as error handling... without any significant advantage. Thanks to this, it gave me an opportunity to think more about using goroutines.

yorkie-team / yorkie