feat: adds `agent config` command that returns the agent configuration

frrist commented 4 weeks ago

closes #4113

Example:

$ bacalhau agent config

API:
    Host: 0.0.0.0
    Port: 1234
    Auth:
        Methods:
            ClientKey:
                Type: challenge
NameProvider: puuid
DataDir: /home/frrist/.bacalhau
Orchestrator:
    Enabled: true
    Host: 0.0.0.0
    Port: 4222
    NodeManager:
        DisconnectTimeout: 1m0s
    Scheduler:
        WorkerCount: 32
        QueueBackoff: 1m0s
        HousekeepingInterval: 30s
        HousekeepingTimeout: 2m0s
    EvaluationBroker:
        VisibilityTimeout: 1m0s
        MaxRetryCount: 10
Compute:
    Orchestrators:
        - nats://127.0.0.1:4222
    Heartbeat:
        InfoUpdateInterval: 1m0s
        ResourceUpdateInterval: 30s
        Interval: 15s
    AllocatedCapacity:
        CPU: 70%
        Memory: 70%
        Disk: 70%
        GPU: 100%
    AllowListedLocalPaths: []
WebUI:
    Listen: 0.0.0.0:8438
InputSources:
    ReadTimeout: 5m0s
    MaxRetryCount: 3
Publishers:
    Types:
        Local:
            Address: 127.0.0.1
            Port: 6001
Engines:
    Types:
        Docker:
            ManifestCache:
                Size: 1000
                TTL: 1h0m0s
                Refresh: 1h0m0s
JobDefaults:
    Batch:
        Task:
            Resources:
                CPU: 500m
                Memory: 1Gb
    Ops:
        Task:
            Resources:
                CPU: 500m
                Memory: 1Gb
    Daemon:
        Task:
            Resources:
                CPU: 500m
                Memory: 1Gb
    Service:
        Task:
            Resources:
                CPU: 500m
                Memory: 1Gb
JobAdmissionControl:
    Locality: anywhere
Logging:
    Level: info
    Mode: default
    LogDebugInfoInterval: 30s
UpdateConfig:
    Interval: 24h0m0s

Summary by CodeRabbit

New Features
- Introduced a command to retrieve the agent's configuration, allowing users to access configuration details easily.
- Added support for customizable output formats (JSON and YAML) for the configuration command.
Bug Fixes
- Enhanced error handling and sensitive data redaction in the configuration retrieval process.
Tests
- Implemented tests to ensure the correct functionality and output of the new configuration command, including validation of sensitive data redaction.

frrist commented 4 weeks ago

It looks like this PR has broken the /api/v1/agent/config endpoint. The response is now the binary encoding of the config {"Config":"QVBJOgogICAgSG9zdDogMC4wLjAuMAogICAgUG9yd..... . I know about that since some of the TestContainers tests are failing

Yes, this is to be expected given the body of the request is now []byte. My aim here is to simplify the response type, as it stand right now, the body contains a yaml string reflecting the servers current configuration. Clients need only print the body, rather than try to interpret the body. Open to alternatives here. I suppose one alternative may be to return an actual string, instead of a binary encoded one.

Are there any potential sensitive data that we might need to mask from that config? Maybe any connection strings with passwords in them, or similar items ?

Yes, several fields of the config may contain tokens, for example the Compute config here. My personal opinion is that config files shouldn't contain sensitive data - it should live somewhere else. That point aside, I am not sure excluding them from the (returned) config is the right choice, perhaps it may be simpler to require auth to query this endpoint? Again, open to alternatives.

Should we add few tests to protect this feature

100%. Can you point me to the appropriate package to writing such tests?

wdbaruni commented 3 weeks ago

The API should just return json representation of the config exactly the same how it works today. No binary and no need for nesting inside a Config field unless we think more fields can be returned, which I doubt it is the case. Parsing into yaml representation should only happen in the CLI. It should just work with curl localhost:1234/api/v1/agent/config

I agree with Jamil about having to mask sensitive data. The only fields we have today are Compute.Auth.Token and Orchestrator.Auth.Token. If running in debug mode, then we can return the values as they are. Otherwise we should mask them. The maintainable approach would be to have a dedicated field type for sensitive data instead of treating them as plain strings, and let the masking take place in one location when mashalling those fields. But that is a lot of work for now and the quick solution would be to mask those fields in the API handler or when mashalling the root config type

jamlo commented 3 weeks ago

Yes, this is to be expected given the body of the request is now []byte. My aim here is to simplify the response type, as it stand right now, the body contains a yaml string reflecting the servers current configuration. Clients need only print the body, rather than try to interpret the body. Open to alternatives here. I suppose one alternative may be to return an actual string, instead of a binary encoded one.

I think it better to return the json representation of the YAML config, since it is much more standard API behavior. Although Json is a subset of YAML, and not the other way around, I believe the current structure of the config should convert to JSON with now issues.

Yes, several fields of the config may contain tokens, for example the Compute config here. My personal opinion is that config files shouldn't contain sensitive data - it should live somewhere else. That point aside, I am not sure excluding them from the (returned) config is the right choice, perhaps it may be simpler to require auth to query this endpoint? Again, open to alternatives.

I think requiring authentication is for sure needed for this endpoint, regardless if it contains sensitive data (exposing configuration publicly is a big security attack vector.) Also, I think masking certain types as "redacted" will also minimize that attack vector, and if we set the boundary that no credentials can be even queried through API, a user should be on the node itself to see these creds.

100%. Can you point me to the appropriate package to writing such tests?

I was thinking of a basic unit test that lives beside the cmd/cli/agent/config.go file for now. And I will fix the TestContainers tests when the PR is merged.

frrist commented 3 weeks ago

The API should just return json representation of the config exactly the same how it works today I think it better to return the json representation of the YAML config

Sounds good, will make the change.

the quick solution would be to mask those fields in the API handler or when mashalling the root config type I think masking certain types as "redacted" will also minimize that attack vector

Also sounds good, will change.

I was thinking of a basic unit test that lives beside the cmd/cli/agent/config.go file for now. And I will fix the TestContainers tests when the PR is merged.

Sounds good to me.

frrist commented 3 weeks ago

@wdbaruni @jamlo how do we want the client/CLI to display the config to the users? Should it marshal the JSON payload to YAML?

jamlo commented 3 weeks ago

@wdbaruni @jamlo how do we want the client/CLI to display the config to the users? Should it marshal the JSON payload to YAML?

I've noticed that some commands already have this flag:

--output format            The output format for the command (one of ["table" "csv" "json" "yaml"]) (default table)

Maybe in this situation csv and table does not apply. But we can give the user the option to choose the format, and default to any format you see fit (I prefer YAML since it is easier to read).

frrist commented 3 weeks ago

@jamlo in fe291ae106af3e6a14e1c307b77048f475b0aa50 I have (attempted to) fix the integration tests. I've been unable to verify this locally as the (new) integration tests fail on my Ubuntu host with the following error message:

2024/10/30 14:02:45 ====> Starting the whole test flow:  a0c4d041
2024/10/30 14:02:45 Error compiling the bacalhau binary:  compiling bacalhau: failed to start container: create container: Error response from daemon: client version 1.45 is too new. Maximum supported API version is 1.43

I suspect I either need to downgrade my client version or upgrade the version required by the testing framework. wdyt?

jamlo commented 3 weeks ago

@frrist For the docker API version, you can configure that in the Tests when you run it locally here . I've set the APi version since Github Actions does not have the latest version, so I pin it.

Btw, I checked the TestContainers tests, and they are running fine on CI after you did the changes, except for one Test that is failing due to how I am calling curl (I should call it to only output the response body).

Since these tests are not a blocker to merge yet, I will fix it when you merge the PR.

coderabbitai[bot] commented 3 weeks ago

Walkthrough

A new command for retrieving the agent's configuration has been added to the Bacalhau project. This includes the implementation of a Cobra command in cmd/cli/agent/config.go, which sets up the repository configuration and creates an API client to fetch the agent's configuration. The changes also introduce a corresponding test suite in cmd/cli/agent/config_test.go to validate the command's output in both JSON and YAML formats. Additionally, modifications in several files enhance the API's ability to manage and return the agent's configuration, including error handling and sensitive data redaction.

Changes

File Path	Change Summary
cmd/cli/agent/config.go	Added `NewConfigCmd` function for the config command and `run` function for executing the API call.
cmd/cli/agent/config_test.go	Introduced `ConfigSuite` for testing JSON and YAML output of the configuration command.
cmd/cli/agent/root.go	Integrated `NewConfigCmd` into the command hierarchy.
pkg/config/types/bacalhau.go	Added a comment for the `DisableAnalytics` field in the `Bacalhau` struct.
pkg/publicapi/apimodels/agent.go	Introduced `GetAgentConfigResponse` type for encapsulating configuration details in API responses.
pkg/publicapi/client/v2/api_agent.go	Added `Config` method to retrieve the agent's configuration.
pkg/publicapi/endpoint/agent/endpoint.go	Enhanced `config` method to include error handling and redact sensitive information.
pkg/publicapi/endpoint/agent/endpoint_test.go	Introduced `TestEndpointConfigRedactFields` to test sensitive data redaction in API responses.
test_integration/1_orchestrator_basic_config_suite_test.go	Modified test logic to adapt to changes in JSON structure.
test_integration/2_orchestrator_config_override_suite_test.go	Adjusted import order; no functional changes.
test_integration/3_orchestrator_config_override_and_flag_suite_test.go	Adjusted import order; no functional changes.
test_integration/4_orchestrator_config_override_and_flag_and_config_flag_suite_test.go	Modified test logic to adapt to changes in JSON structure.
test_integration/5_orchestrator_no_config_suite_test.go	Modified test logic to adapt to changes in JSON structure.
test_integration/6_jobs_basic_runs_scenarios_suite_test.go	Adjusted import order; no functional changes.

Assessment against linked issues

Objective	Addressed	Explanation
API to retrieve node config (#4113)	✅

🐰 In the meadow, where bunnies hop,
A command was added, now we can't stop!
Configurations retrieved with a cheer,
JSON and YAML, all crystal clear.
With tokens redacted, our secrets stay tight,
Hooray for the changes that make things right! 🌼

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 , please review it.` - `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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@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 using 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. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### 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](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.

bacalhau-project / bacalhau