feat: add ,inline tag for toml mapping

[zyy17]

What's changed

1. Add ,inline tag for toml mapping;
2. Add k8sutil.GetSecretsData() to unify the code logic of configuring object storage credentials;
3. Use k8s.io/utils/pointer package to get the pointer of data;

Summary by CodeRabbit

Release Notes

• New Features

  • Introduced a new configuration framework for storage and Write-Ahead Logging (WAL) in the database context.
  • Added a function to retrieve specific data from Kubernetes secrets.

• Improvements

  • Refactored configuration handling for Datanode and Standalone components to enhance clarity and maintainability.
  • Standardized the method of setting replica counts across various components.
  • Consolidated storage configuration fields into a dedicated StorageConfig structure.

• Bug Fixes

  • Removed outdated utility functions to streamline the codebase.

• Dependency Updates

  • Updated dependency management in the project to reflect changes in required libraries.

[coderabbitai]

Warning

Rate limit exceeded

@zyy17 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 53 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between 7169c8a and 873762d.

Walkthrough

The changes encompass updates across multiple files, primarily focusing on refactoring configuration management in a GreptimeDB cluster context. Key modifications include introducing a new interface for object storage access, replacing utility functions for creating pointers with those from the k8s.io/utils/pointer package, restructuring configuration structures, and enhancing the handling of storage and Write-Ahead Logging (WAL) settings. Several utility functions were removed, and new methods for managing Kubernetes secrets were introduced.

Changes

Files	Change Summary
apis/v1alpha1/common.go	Added ObjectStorageProviderAccessor interface with methods for accessing various storage types; included type assertion for ObjectStorageProviderSpec.
cmd/initializer/internal/config_generator.go	Replaced util.StringPtr with pointer.String for string pointer creation in generateDatanodeConfig and generateFlownodeConfig methods.
controllers/greptimedbcluster/controller_test.go	Updated replica counts in FrontendSpec, MetaSpec, and DatanodeSpec from proto.Int32 to pointer.Int32.
go.mod	Modified dependencies: removed gogo/protobuf, adjusted protobuf, and added k8s.io/utils.
pkg/dbconfig/common.go	Introduced StorageConfig and WALConfig structures for managing storage and WAL configurations, with methods for S3, OSS, and GCS setup.
pkg/dbconfig/datanode_config.go	Refactored DatanodeConfig to utilize StorageConfig, simplifying configuration management and updating pointer creation methods.
pkg/dbconfig/dbconfig.go	Updated configuration handling logic: changed return type in NewFromComponentKind, renamed setConfig to mergeConfig, and removed functions for service account and OCS credential management.
pkg/dbconfig/meta_config.go	Added MetaConfig struct with methods for configuring based on cluster context.
pkg/dbconfig/metasrv_config.go	Deleted MetasrvConfig struct and its associated methods.
pkg/dbconfig/standalone_config.go	Modified StandaloneConfig to embed StorageConfig and WALConfig, refactored configuration methods for clarity.
pkg/util/k8s/k8s.go	Added GetSecretsData function to retrieve data from Kubernetes secrets based on specified keys.
pkg/util/util.go	Removed utility functions StringPtr, BoolPtr, and Uint64Ptr.
tests/e2e/greptimedbcluster/test_scale_cluster.go	Updated TestScaleCluster to use pointer.Int32 for setting replica counts in testCluster.

Possibly related PRs

• chore: bump dependencies package #175: The changes in this PR involve updating the go.mod file to include the k8s.io/utils package, which is relevant to the main PR's changes that replace util.StringPtr with pointer.String from the same package.

Poem

🐇 In the land of code, where changes bloom,
New pointers arise, dispelling the gloom.
Configs refined, like carrots so bright,
With each little tweak, our code takes flight!
Hopping through functions, so neat and so spry,
A rabbit rejoices, as the old ways say goodbye! 🌼

---

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
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  -- I pushed a fix in commit <commit_id>, 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 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 for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 53816a2 and d19a29c.

Files selected for processing (12)

• cmd/initializer/internal/config_generator.go (3 hunks)
• controllers/greptimedbcluster/controller_test.go (2 hunks)
• go.mod (3 hunks)
• pkg/dbconfig/common.go (1 hunks)
• pkg/dbconfig/datanode_config.go (2 hunks)
• pkg/dbconfig/dbconfig.go (5 hunks)
• pkg/dbconfig/meta_config.go (1 hunks)
• pkg/dbconfig/metasrv_config.go (0 hunks)
• pkg/dbconfig/standalone_config.go (2 hunks)
• pkg/util/k8s/k8s.go (2 hunks)
• pkg/util/util.go (0 hunks)
• tests/e2e/greptimedbcluster/test_scale_cluster.go (3 hunks)

Files not reviewed due to no reviewable changes (2)

• pkg/dbconfig/metasrv_config.go
• pkg/util/util.go

Additional comments not posted (38)

pkg/dbconfig/meta_config.go (5)

26-41: LGTM!

The MetaConfig struct is well-defined with appropriate fields and types. The use of tomlmapping tags for TOML configuration mapping is a good practice.

---

44-63: LGTM!

The ConfigureByCluster method correctly sets the meta configuration fields based on the given GreptimeDBCluster. The use of the pointer package for creating pointers is a good practice.

---

66-68: LGTM!

The empty implementation of ConfigureByStandalone is acceptable since it's not needed in cluster mode.

---

71-73: LGTM!

The Kind method correctly returns the component kind of the meta.

---

76-84: LGTM!

The GetInputConfig and SetInputConfig methods are straightforward and serve their intended purpose of getting and setting the input config of the meta.

go.mod (3)

22-22: LGTM!

The addition of the k8s.io/utils dependency to the main require block aligns with the PR objective of utilizing the k8s.io/utils/pointer package. The specific version being used is clearly specified.

---

42-42: Looks good!

Moving the github.com/gogo/protobuf dependency to the indirect section is a valid change, as it is not being used directly in the code but is required by one of the direct dependencies. This change is consistent with the dependency management updates mentioned in the PR summary.

---

86-86: LGTM!

Relocating the google.golang.org/protobuf dependency to the indirect section is a valid change, as it is not being used directly in the code but is required by one of the direct dependencies. This change aligns with the dependency management updates outlined in the PR summary.

pkg/util/k8s/k8s.go (1)

138-159: LGTM!

The GetSecretsData function is well-implemented and provides a convenient way to retrieve data associated with specific keys from a Kubernetes secret. Here are some positive aspects of the implementation:

• The function handles error scenarios gracefully by returning an error if the secret is not found, the secret's data is nil, or if any of the specified keys are missing.
• It encapsulates the logic of retrieving the secret and checking for the presence of keys, promoting code reuse and maintainability.
• The function is well-structured and follows a clear logic flow.
• The error messages are informative and provide sufficient context.
• The function name, parameter names, and variable names are descriptive and meaningful.
• The function returns the secret data as a slice of byte slices, allowing flexibility in further processing the data.

Overall, the GetSecretsData function is a valuable addition to the codebase and enhances the functionality related to retrieving data from Kubernetes secrets.

pkg/dbconfig/common.go (5)

27-39: LGTM!

The StorageConfig struct provides a comprehensive set of fields to configure storage settings. The use of pointer fields allows for optional configuration, and the tomlmapping tags enable mapping the fields to TOML configuration.

---

42-63: LGTM!

The ConfigureS3 method correctly maps the S3 storage fields to the StorageConfig fields. It handles the optional secret name by retrieving the access keys from Kubernetes secrets using k8sutil.GetSecretsData, and the error handling for retrieving secrets is appropriate.

---

66-87: LGTM!

The ConfigureOSS method correctly maps the OSS storage fields to the StorageConfig fields. It handles the optional secret name by retrieving the access keys from Kubernetes secrets using k8sutil.GetSecretsData, and the error handling for retrieving secrets is appropriate.

---

90-114: LGTM!

The ConfigureGCS method correctly maps the GCS storage fields to the StorageConfig fields. It handles the optional secret name by retrieving the service account key from Kubernetes secrets using k8sutil.GetSecretsData and encoding it as a base64 string. The error handling for retrieving secrets is appropriate.

---

117-126: LGTM!

The WALConfig struct provides fields to configure the WAL settings, including the directory, provider, and Kafka broker endpoints. The use of pointer fields allows for optional configuration, and the tomlmapping tags enable mapping the fields to TOML configuration. The comments provide clear descriptions of each field's purpose.

pkg/dbconfig/dbconfig.go (4)

27-32: LGTM!

The new constants TOMLMappingFieldTag and InlinedFieldTag provide a clean way to customize the TOML representation of struct fields. The InlinedFieldTag is particularly useful for flattening nested struct fields in the TOML.

---

57-57: LGTM!

The change to return a MetaConfig instance for the MetaComponentKind case aligns with the updated return type of the NewFromComponentKind function to return a Config interface.

---

78-78: LGTM!

The change to call the mergeConfig function instead of setConfig indicates a renaming of the function. The mergeConfig function likely merges the input TOML tree with the provided config struct, which is a clearer name for its functionality.

---

Line range hint 114-178: LGTM!

The mergeConfig function is a well-designed and robust implementation for merging the input TOML tree with the provided config struct. The renaming clarifies its purpose, and the handling of inlined fields, various field types, and error cases improves its functionality and reliability.

cmd/initializer/internal/config_generator.go (4)

117-117: LGTM!

The change to use pointer.String instead of util.StringPtr is consistent with the PR objective and looks good.

---

124-125: LGTM!

The change to use pointer.String instead of util.StringPtr is consistent with the PR objective and looks good. The formatting change to split the string value across two lines is also fine.

---

160-160: LGTM!

The change to use pointer.String is consistent with the PR objective and the similar changes made in the generateDatanodeConfig function. It looks good.

---

167-168: LGTM!

The change to use pointer.String is consistent with the PR objective and the similar changes made in the generateDatanodeConfig function. The formatting change to split the string value across two lines is also fine. It looks good.

tests/e2e/greptimedbcluster/test_scale_cluster.go (2)

82-83: LGTM!

The change to use pointer.Int32 instead of proto.Int for setting replica counts is a good improvement. It makes the code more consistent with Kubernetes ecosystem conventions and improves readability.

---

116-117: LGTM!

Similar to the previous segment, the change to use pointer.Int32 instead of proto.Int for setting replica counts during scale down is a good improvement. It makes the code more consistent with Kubernetes ecosystem conventions and improves readability.

controllers/greptimedbcluster/controller_test.go (3)

129-129: LGTM!

The change from proto.Int32(1) to pointer.Int32(1) is consistent with the import of the k8s.io/utils/pointer package and does not alter the functionality.

---

134-134: LGTM!

The change from proto.Int32(1) to pointer.Int32(1) is consistent with the import of the k8s.io/utils/pointer package and does not alter the functionality.

---

142-142: LGTM!

The change from proto.Int32(3) to pointer.Int32(3) is consistent with the import of the k8s.io/utils/pointer package and does not alter the functionality.

pkg/dbconfig/standalone_config.go (4)

27-31: Good use of embedded structs with inline TOML mapping

Embedding StorageConfig and WALConfig into StandaloneConfig with tomlmapping:",inline" simplifies the configuration structure and enhances code readability. This approach promotes better modularity and maintainability.

---

38-40: Correctly indicating unused parameters using underscore

Using an underscore _ for the unused cluster parameter in the ConfigureByCluster method clearly communicates that this parameter is intentionally not used. This is a good practice for code clarity.

---

64-68: Consistent use of pointer.String enhances code consistency

Replacing custom pointer utility functions with pointer.String from k8s.io/utils/pointer standardizes pointer handling across the codebase. This change improves code consistency and aligns with common Go practices.

---

77-80: Proper handling of Kafka WAL configuration

The addition of Kafka WAL configuration is correctly implemented by checking for its presence and setting WalProvider and WalBrokerEndpoints accordingly. This ensures flexibility and proper configuration when Kafka WAL is used.

pkg/dbconfig/datanode_config.go (7)

18-18: Use of k8s.io/utils/pointer enhances consistency

Replacing custom pointer utilities with the standard pointer package from k8s.io/utils improves code consistency and readability across the codebase.

---

31-32: Embedding StorageConfig inline improves modularity

By embedding StorageConfig inline into DatanodeConfig, storage-related configurations are encapsulated within a dedicated struct. This enhances code organization and maintainability.

---

34-35: Including WALConfig inline enhances code structure

Embedding WALConfig inline into DatanodeConfig effectively organizes WAL-related configurations, promoting better modularity and clarity.

---

63-63: Consistent use of pointer.String for setting WalDir

Using pointer.String to set c.WalDir aligns with the updated pointer utility usage, ensuring consistency across the codebase.

---

66-67: Properly setting StorageDataHome when data home is specified

The assignment of c.StorageDataHome using pointer.String(dataHome) correctly sets the storage data home directory when specified in the cluster configuration.

---

70-71: Effective error handling in SetInputConfig

Ensuring that any error returned by c.SetInputConfig(cfg) is appropriately handled and returned maintains robust error propagation and reliability.

---

76-78: Correct configuration of Kafka WAL provider

Successfully sets the WAL provider to "kafka" and assigns the broker endpoints when Kafka WAL is enabled, ensuring that the data node is configured correctly for Kafka-based WAL.

[daviderli614]

LGTM