Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dag inspector #65

Open
wants to merge 90 commits into
base: main
Choose a base branch
from
Open

Dag inspector #65

wants to merge 90 commits into from

Conversation

welbon
Copy link
Collaborator

@welbon welbon commented Jun 18, 2024

Summary by CodeRabbit

  • New Features

    • Introduced a new configuration for Elasticsearch with enhanced security and snapshot management in Kubernetes.
    • Added a new Kubernetes deployment for Elasticsearch, including resource management and initialization processes.
    • Implemented Persistent Volume and Volume Snapshot configurations for data retention in Elasticsearch.
    • Established a new service configuration for Elasticsearch to manage traffic routing.
    • Created additional Persistent Volume Claims for improved storage management.
    • Introduced a new deployment for the starcoin-indexer-stcscan application with secure handling of environment variables.
  • Chores

    • Updated environment variables for Elasticsearch connections to utilize cluster-local services, enhancing reliability and security.
    • Adjusted environment variables to reference secrets for sensitive information, improving security practices.

fountainchen and others added 30 commits August 11, 2022 22:29
(cherry picked from commit b0201760a5dffdf81b5a12f7a061e682ec14aa1f)
…een this logic and Kaspa Processing is that it first takes part of the block, and then calculates the inspection drawing information based on the data of this part of the block.

(cherry picked from commit 2aadab7662ff43114247db2b1674811feadf9836)
…dling, that is, `com.thetransactioncompany.jsonrpc2.client.JSONRPC2SessionException` should be `org.starcoin.jsonrpc.client.JSONRPC2SessionException`;

(cherry picked from commit b568d235e689a643ab138c59ae8aa79439da5c6f)
…o the API

(cherry picked from commit 55d22454e1add961a15608ff8531ad0abebceea8)
(cherry picked from commit 520277f1679a7d62f914548937a87633d3f3cfef)
…sitory

(cherry picked from commit 1e75f0657ee6c20f181d95958f341df9395886f5)
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 42e7009 and 4fe719d.

Files selected for processing (3)
  • kube/base-components/allowaccess-network-policy.yaml (1 hunks)
  • kube/base-components/kibana-deployment.yaml (1 hunks)
  • kube/indexer/stc-scan/stcscan-vega-deployment.yaml (1 hunks)
Additional context used
yamllint
kube/base-components/kibana-deployment.yaml

[warning] 37-37: wrong indentation: expected 16 but found 15

(indentation)


[error] 74-74: no new line character at the end of file

(new-line-at-end-of-file)

Additional comments not posted (4)
kube/base-components/kibana-deployment.yaml (2)

37-37: Fix indentation

The indentation is incorrect for the secretKeyRef key. It should be 16 spaces.

-               secretKeyRef:
+                secretKeyRef:
Tools
yamllint

[warning] 37-37: wrong indentation: expected 16 but found 15

(indentation)


74-74: Add newline at the end of the file

A newline character is missing at the end of the file.

-      nodePort: 31000
+      nodePort: 31000
+
Tools
yamllint

[error] 74-74: no new line character at the end of file

(new-line-at-end-of-file)

kube/base-components/allowaccess-network-policy.yaml (1)

70-72: Fix indentation and add a newline at the end of the file.

Address the indentation warnings and add a newline at the end of the file.

-      - namespaceSelector:
-         matchLabels:
-          name: starcoin-api
+        - namespaceSelector:
+            matchLabels:
+              name: starcoin-api

+ # Add a newline at the end of the file
kube/indexer/stc-scan/stcscan-vega-deployment.yaml (1)

1-67: Ensure sensitive information is managed securely

The environment variables STARCOIN_ES_PWD, DB_USER_NAME, and DB_PWD are being sourced from Kubernetes secrets, which is a good practice. Ensure that these secrets are securely managed and rotated regularly.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 4fe719d and 5c04432.

Files selected for processing (18)
  • kube/indexer/stc-scan/stcscan-barnard-deployment.yaml (2 hunks)
  • kube/indexer/stc-scan/stcscan-cmd-handle-main-deployment.yaml (2 hunks)
  • kube/indexer/stc-scan/stcscan-halley-deployment.yaml (1 hunks)
  • kube/indexer/stc-scan/stcscan-main-deployment.yaml (2 hunks)
  • kube/indexer/stc-scan/stcscan-proxima-deployment.yaml (2 hunks)
  • kube/indexer/stc-scan/stcscan-repair-barnard-deployment.yaml (2 hunks)
  • kube/indexer/stc-scan/stcscan-repair-halley-deployment.yaml (1 hunks)
  • kube/indexer/stc-scan/stcscan-repair-main-deployment.yaml (1 hunks)
  • kube/indexer/stc-scan/stcscan-txn-main-deployment.yaml (2 hunks)
  • kube/indexer/swap/swap-info-main-deployment.yaml (1 hunks)
  • kube/indexer/swap/swap-stat-main-deployment.yaml (2 hunks)
  • kube/indexer/swap/swap-txns-main-deployment.yaml (3 hunks)
  • kube/subscribe/starscan-sub-barnard-deployment-ali.yaml (1 hunks)
  • kube/subscribe/starscan-sub-barnard-deployment.yaml (1 hunks)
  • kube/subscribe/starscan-sub-halley-deployment.yaml (1 hunks)
  • kube/subscribe/starscan-sub-main-deployment-ali.yaml (1 hunks)
  • kube/subscribe/starscan-sub-main-deployment.yaml (1 hunks)
  • kube/subscribe/starscan-sub-proxima-deployment.yaml (1 hunks)
Files skipped from review as they are similar to previous changes (12)
  • kube/indexer/stc-scan/stcscan-barnard-deployment.yaml
  • kube/indexer/stc-scan/stcscan-halley-deployment.yaml
  • kube/indexer/stc-scan/stcscan-main-deployment.yaml
  • kube/indexer/stc-scan/stcscan-proxima-deployment.yaml
  • kube/indexer/stc-scan/stcscan-repair-halley-deployment.yaml
  • kube/indexer/stc-scan/stcscan-repair-main-deployment.yaml
  • kube/indexer/swap/swap-info-main-deployment.yaml
  • kube/indexer/swap/swap-stat-main-deployment.yaml
  • kube/indexer/swap/swap-txns-main-deployment.yaml
  • kube/subscribe/starscan-sub-halley-deployment.yaml
  • kube/subscribe/starscan-sub-main-deployment.yaml
  • kube/subscribe/starscan-sub-proxima-deployment.yaml
Additional context used
yamllint
kube/indexer/stc-scan/stcscan-repair-barnard-deployment.yaml

[error] 69-69: no new line character at the end of file

(new-line-at-end-of-file)

Additional comments not posted (48)
kube/subscribe/starscan-sub-barnard-deployment-ali.yaml (5)

31-31: LGTM! Updated Elasticsearch URL.

The STARCOIN_ES_URL environment variable now uses the Kubernetes service for Elasticsearch, enhancing maintainability and security.


37-41: LGTM! Updated Elasticsearch username to use secrets.

The STARCOIN_ES_USER environment variable now uses a secret reference, enhancing security by storing sensitive information in Kubernetes secrets.


42-45: LGTM! Updated Elasticsearch password to use secrets.

The STARCOIN_ES_PWD environment variable now uses a secret reference, enhancing security by storing sensitive information in Kubernetes secrets.


51-51: LGTM! Updated PostgreSQL URL.

The DS_URL environment variable now uses the Kubernetes service for PostgreSQL, enhancing maintainability and security.


53-56: LGTM! Updated PostgreSQL credentials to use secrets.

The DB_USER_NAME and DB_PWD environment variables now use secret references, enhancing security by storing sensitive information in Kubernetes secrets.

Also applies to: 60-61

kube/subscribe/starscan-sub-barnard-deployment.yaml (5)

31-31: LGTM! Updated Elasticsearch URL.

The STARCOIN_ES_URL environment variable now uses the Kubernetes service for Elasticsearch, enhancing maintainability and security.


37-41: LGTM! Updated Elasticsearch username to use secrets.

The STARCOIN_ES_USER environment variable now uses a secret reference, enhancing security by storing sensitive information in Kubernetes secrets.


42-45: LGTM! Updated Elasticsearch password to use secrets.

The STARCOIN_ES_PWD environment variable now uses a secret reference, enhancing security by storing sensitive information in Kubernetes secrets.


51-51: LGTM! Updated PostgreSQL URL.

The DS_URL environment variable now uses the Kubernetes service for PostgreSQL, enhancing maintainability and security.


53-56: LGTM! Updated PostgreSQL credentials to use secrets.

The DB_USER_NAME and DB_PWD environment variables now use secret references, enhancing security by storing sensitive information in Kubernetes secrets.

Also applies to: 60-61

kube/subscribe/starscan-sub-main-deployment-ali.yaml (5)

31-31: LGTM! Updated Elasticsearch URL.

The STARCOIN_ES_URL environment variable now uses the Kubernetes service for Elasticsearch, enhancing maintainability and security.


37-41: LGTM! Updated Elasticsearch username to use secrets.

The STARCOIN_ES_USER environment variable now uses a secret reference, enhancing security by storing sensitive information in Kubernetes secrets.


42-45: LGTM! Updated Elasticsearch password to use secrets.

The STARCOIN_ES_PWD environment variable now uses a secret reference, enhancing security by storing sensitive information in Kubernetes secrets.


51-51: LGTM! Updated PostgreSQL URL.

The DS_URL environment variable now uses the Kubernetes service for PostgreSQL, enhancing maintainability and security.


53-56: LGTM! Updated PostgreSQL credentials to use secrets.

The DB_USER_NAME and DB_PWD environment variables now use secret references, enhancing security by storing sensitive information in Kubernetes secrets.

Also applies to: 60-61

kube/indexer/stc-scan/stcscan-txn-main-deployment.yaml (5)

4-7: LGTM! Updated deployment name.

The metadata section now reflects the new naming conventions for the deployment.


12-16: LGTM! Updated selector and labels.

The selector and labels sections now match the new deployment name, ensuring consistency.


35-35: LGTM! Updated Elasticsearch URL.

The STARCOIN_ES_URL environment variable now uses the Kubernetes service for Elasticsearch, enhancing maintainability and security.


41-45: LGTM! Updated Elasticsearch credentials to use secrets.

The STARCOIN_ES_USER and STARCOIN_ES_PWD environment variables now use secret references, enhancing security by storing sensitive information in Kubernetes secrets.


55-60: LGTM! Updated PostgreSQL connection details to use services and secrets.

The DS_URL, DB_USER_NAME, and DB_PWD environment variables now use the Kubernetes service and secret references for PostgreSQL, enhancing maintainability and security.

Also applies to: 64-65

kube/indexer/stc-scan/stcscan-cmd-handle-main-deployment.yaml (14)

4-4: LGTM! Deployment name and namespace.

The deployment name and namespace are consistent with the project's naming conventions.


7-7: LGTM! Labels.

The labels are correctly applied and consistent with the project's standards.


19-19: LGTM! Container name.

The container name is consistent with the deployment name and project standards.


35-35: LGTM! Elasticsearch URL.

The Elasticsearch URL is updated to use the local Kubernetes service.


37-37: LGTM! Elasticsearch protocol.

The Elasticsearch protocol is updated to use HTTP.


39-39: LGTM! Elasticsearch port.

The Elasticsearch port is updated to 9200.


41-45: LGTM! Elasticsearch username.

The Elasticsearch username is now retrieved from a Kubernetes secret.


46-49: LGTM! Elasticsearch password.

The Elasticsearch password is now retrieved from a Kubernetes secret.


55-55: LGTM! PostgreSQL URL.

The PostgreSQL URL is updated to use the local Kubernetes service.


57-60: LGTM! PostgreSQL username.

The PostgreSQL username is now retrieved from a Kubernetes secret.


64-64: LGTM! PostgreSQL password.

The PostgreSQL password is now retrieved from a Kubernetes secret.


Line range hint 19-64:
LGTM! Container specification.

The container specification is correct and secure.


41-49: LGTM! Use of secrets.

The use of secretKeyRef for Elasticsearch and PostgreSQL credentials is correct and secure.

Also applies to: 57-64


Line range hint 19-64:
LGTM! Remaining environment variables.

All remaining environment variables are correctly specified.

kube/indexer/stc-scan/stcscan-repair-barnard-deployment.yaml (14)

4-4: LGTM! Deployment name and namespace.

The deployment name and namespace are consistent with the project's naming conventions.


7-7: LGTM! Labels.

The labels are correctly applied and consistent with the project's standards.


19-19: LGTM! Container name.

The container name is consistent with the deployment name and project standards.


35-35: LGTM! Elasticsearch URL.

The Elasticsearch URL is updated to use the local Kubernetes service.


37-37: LGTM! Elasticsearch protocol.

The Elasticsearch protocol is updated to use HTTP.


39-39: LGTM! Elasticsearch port.

The Elasticsearch port is updated to 9200.


41-45: LGTM! Elasticsearch username.

The Elasticsearch username is now retrieved from a Kubernetes secret.


46-49: LGTM! Elasticsearch password.

The Elasticsearch password is now retrieved from a Kubernetes secret.


55-55: LGTM! PostgreSQL URL.

The PostgreSQL URL is updated to use the local Kubernetes service.


57-60: LGTM! PostgreSQL username.

The PostgreSQL username is now retrieved from a Kubernetes secret.


64-64: LGTM! PostgreSQL password.

The PostgreSQL password is now retrieved from a Kubernetes secret.


Line range hint 19-64:
LGTM! Container specification.

The container specification is correct and secure.

Tools
yamllint

[error] 69-69: no new line character at the end of file

(new-line-at-end-of-file)


41-49: LGTM! Use of secrets.

The use of secretKeyRef for Elasticsearch and PostgreSQL credentials is correct and secure.

Also applies to: 57-64


Line range hint 19-69:
LGTM! Remaining environment variables.

All remaining environment variables are correctly specified.

Tools
yamllint

[error] 69-69: no new line character at the end of file

(new-line-at-end-of-file)

key: password
- name: DB_SCHEMA
value: "barnard"
- name: PROGRAM_ARGS
value: "auto_repair 16057420"
value: "auto_repair 16057420"
Copy link

Choose a reason for hiding this comment

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

Fix the missing newline at the end of the file.

Ensure the file ends with a newline character.

-  value: "auto_repair 16057420"
+  value: "auto_repair 16057420"
+
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
value: "auto_repair 16057420"
value: "auto_repair 16057420"
Tools
yamllint

[error] 69-69: no new line character at the end of file

(new-line-at-end-of-file)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5c04432 and 82b0dbd.

Files selected for processing (1)
  • kube/base-components/elasticsearch-deployment.yaml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • kube/base-components/elasticsearch-deployment.yaml

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 82b0dbd and feb9555.

Files selected for processing (4)
  • kube/base-components/allowaccess-network-policy.yaml (1 hunks)
  • kube/base-components/namespaces.yaml (1 hunks)
  • kube/indexer/stc-scan/stcscan-main-deployment.yaml (1 hunks)
  • kube/scan-api/scan-api-deployment.yaml (2 hunks)
Files skipped from review as they are similar to previous changes (2)
  • kube/base-components/allowaccess-network-policy.yaml
  • kube/indexer/stc-scan/stcscan-main-deployment.yaml
Additional context used
yamllint
kube/base-components/namespaces.yaml

[error] 55-55: no new line character at the end of file

(new-line-at-end-of-file)

Additional comments not posted (9)
kube/base-components/namespaces.yaml (5)

1-11: LGTM!

The namespace starcoin-main is defined correctly with appropriate metadata and labels.


12-22: LGTM!

The namespace starcoin-barnard is defined correctly with appropriate metadata and labels.


23-33: LGTM!

The namespace starcoin-proxima is defined correctly with appropriate metadata and labels.


34-44: LGTM!

The namespace starcoin-halley is defined correctly with appropriate metadata and labels.


45-55: LGTM!

The namespace starcoin-vega is defined correctly with appropriate metadata and labels.

Tools
yamllint

[error] 55-55: no new line character at the end of file

(new-line-at-end-of-file)

kube/scan-api/scan-api-deployment.yaml (4)

Line range hint 1-5:
LGTM!

The metadata section is defined correctly with appropriate name and namespace.


9-12: LGTM!

The spec section is defined correctly with appropriate replicas and selector.


Line range hint 13-15:
LGTM!

The template metadata section is defined correctly with appropriate labels.


Line range hint 16-65:
LGTM! But verify the usage of environment variables.

The container definition is correct with appropriate image, ports, and environment variables.

However, ensure that the environment variables, especially those marked as "maybe useless," are actually needed.

Verification successful

Let's correct the file type specification and re-run the script to verify the usage of the environment variables in the codebase.


Let's start by listing all Python files in the repository to confirm their presence. Then, we will search for the environment variables in all file types to ensure we cover all possible usages.


Environment variables are necessary and in use.

The environment variables MAIN_DS_URL, BARNARD_DS_URL, HALLEY_DS_URL, and STARCOIN_USER_DS_URL are used across multiple configuration files, such as application.properties, application-unittest.properties, and local.env. Therefore, they are not useless and are required for the application's configuration.

  • starcoin-scan-api/src/main/resources/application.properties
  • starcoin-scan-api/src/test/resources/application-unittest.properties
  • starcoin-scan-api/local.env
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of environment variables in the codebase.

# Test: Search for the environment variables in the codebase. Expect: Only relevant variables should be used.
rg --type python -A 5 $'MAIN_DS_URL|BARNARD_DS_URL|HALLEY_DS_URL|STARCOIN_USER_DS_URL'

Length of output: 118


Script:

#!/bin/bash
# Description: Verify the usage of environment variables in the codebase.

# Test: Search for the environment variables in the codebase. Expect: Only relevant variables should be used.
rg --type py -A 5 $'MAIN_DS_URL|BARNARD_DS_URL|HALLEY_DS_URL|STARCOIN_USER_DS_URL'

Length of output: 232


Script:

#!/bin/bash
# Step 1: List all Python files in the repository.
echo "Listing all Python files in the repository:"
fd -e py

# Step 2: Search for the environment variables in all file types.
echo "Searching for environment variables in all file types:"
rg -A 5 $'MAIN_DS_URL|BARNARD_DS_URL|HALLEY_DS_URL|STARCOIN_USER_DS_URL'

Length of output: 6563

app: starcoin
kubernetes.io/metadata.name: starcoin-vega
name: starcoin-vega
network: vega
Copy link

Choose a reason for hiding this comment

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

Add a newline at the end of the file.

The file should end with a newline character for better compatibility with various tools.

+ 
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
network: vega
network: vega
Tools
yamllint

[error] 55-55: no new line character at the end of file

(new-line-at-end-of-file)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between feb9555 and 4ba90d0.

Files selected for processing (2)
  • kube/indexer/dag/dag-vega-deployment.yaml (1 hunks)
  • kube/indexer/stc-scan/stcscan-main-deployment.yaml (1 hunks)
Additional comments not posted (6)
kube/indexer/dag/dag-vega-deployment.yaml (3)

3-7: Metadata configuration looks good.

The metadata section is well-defined with appropriate naming and labels.


8-16: Spec configuration is appropriate.

The deployment spec is correctly set up with a single replica and matching labels.


17-64: Container and environment variable configuration looks good.

The container setup is well-defined, and sensitive data is appropriately managed using Kubernetes secrets.

kube/indexer/stc-scan/stcscan-main-deployment.yaml (3)

3-7: Metadata configuration looks good.

The metadata section is well-defined with appropriate naming and labels.


8-16: Spec configuration is appropriate.

The deployment spec is correctly set up with a single replica and matching labels.


17-69: Container and environment variable configuration looks good.

The container setup is well-defined, and sensitive data is appropriately managed using Kubernetes secrets.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 4ba90d0 and d8bbc67.

Files selected for processing (5)
  • starcoin-indexer/src/main/java/org/starcoin/indexer/config/QuartzConfig.java (4 hunks)
  • starcoin-indexer/src/main/java/org/starcoin/indexer/handler/IndexerHandleJob.java (1 hunks)
  • starcoin-indexer/src/main/java/org/starcoin/indexer/handler/LegacyMainIndexHandler.java (3 hunks)
  • starcoin-indexer/src/main/java/org/starcoin/indexer/handler/MarketCapIndexer.java (1 hunks)
  • starcoin-indexer/src/test/java/org/starcoin/indexer/test/IndexHandlerJobTest.java (1 hunks)
Additional comments not posted (9)
starcoin-indexer/src/main/java/org/starcoin/indexer/handler/MarketCapIndexer.java (2)

16-17: Dependency Injection Implemented Correctly

The use of @Autowired for MarketCapHandle ensures that Spring handles its instantiation and dependency resolution, aligning with best practices for Spring applications.


19-19: Proper Use of Dependency Injection

Injecting AddressHolderService with @Autowired is a standard practice in Spring, facilitating better management and decoupling of service dependencies.

starcoin-indexer/src/test/java/org/starcoin/indexer/test/IndexHandlerJobTest.java (1)

1-31: Comprehensive Test Implementation

The test class is well-structured, extending IndexerLogicBaseTest for shared testing functionalities. Dependencies are correctly injected, and the test method testIndexerHandle effectively covers the functionality of LegacyMainIndexHandler.

starcoin-indexer/src/main/java/org/starcoin/indexer/handler/LegacyMainIndexHandler.java (1)

Line range hint 15-183: Refactoring Enhances Clarity and Control

The refactoring of LegacyMainIndexHandler improves the class by removing its dependency on QuartzJobBean, adding explicit dependency management through a constructor, and introducing new methods for initialization and execution. This change enhances maintainability and clarity.

starcoin-indexer/src/main/java/org/starcoin/indexer/handler/IndexerHandleJob.java (2)

164-174: Well-implemented initialization logic.

The initOffset method correctly checks for null before initializing legacyIndexHandler, ensuring that it is only set once. This is a good practice to avoid unnecessary reinitializations.


177-180: Simplified job execution logic.

The executeInternal method is well-implemented, delegating the actual job execution to legacyIndexHandler. This simplification improves the maintainability and readability of the code.

starcoin-indexer/src/main/java/org/starcoin/indexer/config/QuartzConfig.java (3)

27-27: Updated job configuration aligns with new implementation.

The indexerJob method has been correctly updated to use IndexerHandleJob, ensuring that the job configuration is consistent with the new job implementation.


206-219: Correctly configured new job and trigger for DagInspectorIndexer.

The methods dagInspectorJob and dagInspectorTrigger are well-implemented, setting up the new job with a durable store and defining a trigger that executes every 15 seconds indefinitely. This configuration ensures that the new job is properly scheduled and executed.


301-304: Scheduler configuration updated to include new job.

The updates to the scheduler method correctly include the dagInspectorJob and its trigger, ensuring that the new job is integrated into the system's job scheduling. The use of a set to track scheduled jobs is efficient and ensures that jobs are not scheduled multiple times.

Comment on lines +31 to +162
// BlockOffset remoteBlockOffset = elasticSearchHandler.getRemoteOffset();
// logger.info("current remote offset: {}", remoteBlockOffset);
// if (remoteBlockOffset == null) {
// logger.warn("offset must not null, please check blocks.mapping!!");
// return;
// }
// if (remoteBlockOffset.getBlockHeight() > localBlockOffset.getBlockHeight()) {
// logger.info("indexer equalize chain blocks.");
// return;
// }
// //read head
// try {
// BlockHeader chainHeader = blockRPCClient.getChainHeader();
// //calculate bulk size
// long headHeight = chainHeader.getHeight();
// long bulkNumber = Math.min(headHeight - localBlockOffset.getBlockHeight(), bulkSize);
// int index = 1;
// List<Block> blockList = new ArrayList<>();
// while (index <= bulkNumber) {
// long readNumber = localBlockOffset.getBlockHeight() + index;
// Block block = blockRPCClient.getBlockByHeight(readNumber);
// if (!block.getHeader().getParentHash().equals(currentHandleHeader.getBlockHash())) {
// //fork handle until reach forked point block
// logger.warn("Fork detected, roll back: {}, {}, {}", readNumber, block.getHeader().getParentHash(), currentHandleHeader.getBlockHash());
// Block lastForkBlock, lastMasterBlock;
// BlockHeader forkHeader = currentHandleHeader;
// long lastMasterNumber = readNumber - 1;
// String forkHeaderParentHash = null;
// do {
// //获取分叉的block
// if (forkHeaderParentHash == null) {
// //第一次先回滚当前最高的分叉块
// forkHeaderParentHash = forkHeader.getBlockHash();
// } else {
// forkHeaderParentHash = forkHeader.getParentHash();
// }
// lastForkBlock = elasticSearchHandler.getBlockContent(forkHeaderParentHash);
// if (lastForkBlock == null) {
// logger.warn("get fork block null: {}", forkHeaderParentHash);
// //read from node
// lastForkBlock = blockRPCClient.getBlockByHash(forkHeaderParentHash);
// }
// if (lastForkBlock != null) {
// elasticSearchHandler.bulkForkedUpdate(lastForkBlock);
// logger.info("rollback forked block ok: {}, {}", lastForkBlock.getHeader().getHeight(), forkHeaderParentHash);
// } else {
// //如果块取不到,先退出当前任务,下一个轮询周期再执行
// logger.warn("get forked block is null: {}", forkHeaderParentHash);
// return;
// }
//
// //获取上一个高度主块
// lastMasterBlock = blockRPCClient.getBlockByHeight(lastMasterNumber);
// if (lastMasterBlock != null) {
// long forkNumber = forkHeader.getHeight();
// logger.info("fork number: {}", forkNumber);
// forkHeader = lastForkBlock.getHeader();
// //reset offset to handled fork block
// currentHandleHeader = forkHeader;
// localBlockOffset.setBlockHeight(currentHandleHeader.getHeight());
// localBlockOffset.setBlockHash(currentHandleHeader.getBlockHash());
// elasticSearchHandler.setRemoteOffset(localBlockOffset);
// if (lastMasterNumber == forkNumber && lastMasterBlock.getHeader().getBlockHash().equals(forkHeaderParentHash)) {
// //find fork point
// logger.info("find fork height: {}", lastMasterNumber);
// break;
// }
// //继续找下一个分叉
// lastMasterNumber--;
// logger.info("continue last forked block: {}", lastMasterNumber);
// } else {
// logger.warn("get last master Block null: {}", lastMasterNumber);
// }
// } while (true);
//
// logger.info("rollback handle ok: {}", localBlockOffset);
// return; //退出当前任务,重新添加从分叉点之后的block
// }
//
// //set event
// ServiceUtils.fetchTransactionsForBlock(transactionRPCClient, block);
// blockList.add(block);
//
// //update current header
// currentHandleHeader = block.getHeader();
// index++;
// logger.debug("add block: {}", block.getHeader());
// }
// //bulk execute
// elasticSearchHandler.bulk(blockList);
//
// //update offset
// localBlockOffset.setBlockHeight(currentHandleHeader.getHeight());
// localBlockOffset.setBlockHash(currentHandleHeader.getBlockHash());
// elasticSearchHandler.setRemoteOffset(localBlockOffset);
// logger.info("indexer update success: {}", localBlockOffset);
// } catch (JSONRPC2SessionException e) {
// logger.error("chain header error:", e);
// }
// }
Copy link

Choose a reason for hiding this comment

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

Consider removing commented-out code.

The large block of commented-out code could be removed to clean up the class and avoid confusion. If this code is no longer needed due to the refactoring, removing it would improve the maintainability of the file.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Outside diff range and nitpick comments (2)
kube/base-components/elasticsearch-deployment.yaml (2)

25-25: Consider using a specific version tag for the Elasticsearch image.

Using a specific version tag (e.g., 7.17.0) instead of a generic tag (e.g., latest) ensures predictable behavior and makes it easier to manage upgrades. It's a best practice to pin the image version to avoid unexpected changes.


111-120: Consider adding a headless service for Elasticsearch cluster communication.

In addition to the regular service, it's common to create a headless service for Elasticsearch cluster communication. This allows the Elasticsearch nodes to discover each other using DNS.

To create a headless service, you can add the following configuration:

apiVersion: v1
kind: Service
metadata:
  name: elasticsearch-headless
spec:
  clusterIP: None
  selector:
    app: elasticsearch
  ports:
    - port: 9300
      name: transport
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d8bbc67 and b20fb4d.

Files selected for processing (1)
  • kube/base-components/elasticsearch-deployment.yaml (1 hunks)
Additional comments not posted (5)
kube/base-components/elasticsearch-deployment.yaml (5)

1-120: LGTM!

The Elasticsearch deployment configuration looks comprehensive and well-structured. It includes essential components such as resource requests/limits, data persistence using PVCs, security configurations, and a service for exposing the Elasticsearch instance.


16-22: Verify the necessity of the init container.

The init container is used to set the correct permissions for the Elasticsearch data directory. While this is a good practice, it's worth verifying if the Elasticsearch image already handles the permissions correctly. If it does, the init container may be unnecessary.

To verify, you can check the Elasticsearch image documentation or run the container without the init container and see if there are any permission-related issues.


26-32: Review the resource requests and limits.

The current configuration sets resource requests and limits for the Elasticsearch container. While the values seem reasonable, it's important to review them based on your specific workload requirements and the available resources in your cluster.

Monitor the Elasticsearch performance and resource usage after deployment and adjust the values if necessary.


77-80: Verify the Elasticsearch configuration.

The ConfigMap includes settings for enabling security, setting the network host, and specifying the snapshot repository path. Double-check that these configurations align with your requirements and best practices.

You can refer to the Elasticsearch documentation to ensure the settings are appropriate for your use case.


49-58: Ensure the Elasticsearch credentials are securely managed.

The Elasticsearch username and password are being sourced from a Kubernetes secret named elasticsearch. Make sure this secret is created securely and not checked into version control.

You can use the following command to check if the secret exists:

Comment on lines 84 to 108
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
name: es-pvc
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 500Gi
storageClassName: do-block-storage-retain

---
# Elasticsearch Snapshot Repository Persistent Volume Claim
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
name: es-backup-pvc
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 500Gi
storageClassName: do-block-storage-retain
Copy link

Choose a reason for hiding this comment

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

Consider using a storage class that supports resizing.

The PersistentVolumeClaims are using the do-block-storage-retain storage class, which may not support dynamic resizing. If you anticipate needing to increase the storage size in the future, consider using a storage class that supports volume expansion.

Investigate if your storage provider offers a storage class with resizing capabilities and update the PVCs accordingly.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Outside diff range and nitpick comments (2)
kube/base-components/elasticsearch-deployment.yaml (2)

28-34: Consider adjusting resource limits based on actual usage.

The current resource configuration allocates 1 CPU core and 4 GiB of memory for the Elasticsearch container, with an upper limit of 2 CPU cores and 6 GiB of memory. While these values provide a starting point, it's essential to monitor the actual resource usage of Elasticsearch in your specific deployment.

Analyze the CPU and memory utilization of the Elasticsearch container using tools like Kubernetes Metrics Server or Prometheus. Based on the observed usage patterns, adjust the resources.requests and resources.limits values accordingly. Strike a balance between allocating sufficient resources for optimal performance and avoiding overallocation, which can lead to underutilized resources in the cluster.


119-122: Consider using a headless service for Elasticsearch cluster formation.

If you plan to scale the Elasticsearch deployment to multiple nodes in the future, it's recommended to use a headless service. A headless service allows direct access to individual Elasticsearch pods, facilitating cluster formation and inter-node communication.

To create a headless service, add the clusterIP: None field to the service specification:

spec:
  clusterIP: None
  ports:
    - port: 9200
  selector:
    app: elasticsearch

This change ensures that the service does not provide load balancing but instead returns the IP addresses of the associated Elasticsearch pods. Each pod will be directly accessible within the cluster using its unique DNS entry.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b20fb4d and 258f43c.

Files selected for processing (1)
  • kube/base-components/elasticsearch-deployment.yaml (1 hunks)
Additional context used
checkov
kube/base-components/elasticsearch-deployment.yaml

[HIGH] 2-71: Container should not be privileged

(CKV_K8S_16)

Additional comments not posted (2)
kube/base-components/elasticsearch-deployment.yaml (2)

16-22: Ensure the init container has the necessary permissions.

The init container is responsible for adjusting the ownership of the Elasticsearch data directory. Verify that the user and group IDs (1000:1000) specified in the chown command match the user running the Elasticsearch process within the main container. If there's a mismatch, Elasticsearch may encounter permission issues when accessing its data directory.


79-82: Verify the network settings and repository paths.

The Elasticsearch configuration enables security features, sets the network host to listen on all interfaces, and specifies the path for snapshot repositories. Double-check the following:

  1. Ensure that the network settings align with your desired Elasticsearch access patterns. If Elasticsearch should only be accessible within the cluster, consider using a more restrictive network configuration.

  2. Confirm that the specified path for snapshot repositories (/data/es_snapshot_repository) matches the mount path of the corresponding volume in the Elasticsearch container.

  3. Verify that the necessary plugins and configurations are in place to support the snapshot repository functionality.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 9a804b6 and b1633d2.

📒 Files selected for processing (1)
  • kube/base-components/elasticsearch-deployment.yaml (1 hunks)
🧰 Additional context used
🪛 checkov
kube/base-components/elasticsearch-deployment.yaml

[HIGH] 2-95: Container should not be privileged

(CKV_K8S_16)

🔇 Additional comments (4)
kube/base-components/elasticsearch-deployment.yaml (4)

64-81: S3 configuration setup looks good

The S3 configuration for the snapshot repository is well-implemented. It correctly uses secrets for AWS credentials and sets up the necessary environment variables. The lifecycle postStart hook for installing and configuring the S3 plugin is a good approach.


95-111: Review network.host setting

The current configuration sets network.host: 0.0.0.0, which allows Elasticsearch to bind to all available network interfaces. While this can be convenient, it might expose Elasticsearch to more networks than necessary.

Consider if this broad network access is required for your use case. If not, it's recommended to restrict it to specific interfaces or IP addresses for improved security. For example:

network.host: _site_

This setting binds Elasticsearch to the site-local addresses.

Please verify your network requirements and adjust this setting accordingly.


140-150: Service configuration looks good

The Elasticsearch Service is correctly configured to expose port 9200 and uses the appropriate selector to match the Elasticsearch pod.


1-150: Overall assessment: Comprehensive Elasticsearch setup with some security considerations

This file provides a well-structured Kubernetes deployment for Elasticsearch, including necessary components such as ConfigMap, PersistentVolumeClaims, and Service. The configuration is suitable for a production environment with considerations for data persistence and S3 integration.

Key points:

  1. The privileged mode issue needs to be addressed for improved security.
  2. Consider using a dedicated secret for Elasticsearch credentials.
  3. Review the network host setting in the ConfigMap.
  4. Investigate storage class options for future resizing capabilities.

Once these points are addressed, this setup should provide a robust and secure Elasticsearch deployment in your Kubernetes environment.

🧰 Tools
🪛 checkov

[HIGH] 2-95: Container should not be privileged

(CKV_K8S_16)

Comment on lines 25 to 26
securityContext:
privileged: true
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

[HIGH] Remove privileged mode for the Elasticsearch container

Running containers with privileged: true grants them extensive system-level access, potentially compromising the security of the Kubernetes cluster. This is a significant security risk.

Instead of using privileged mode, identify the specific capabilities required by Elasticsearch and use securityContext.capabilities to add only those. For example:

securityContext:
  capabilities:
    add:
      - IPC_LOCK
      - SYS_RESOURCE

This approach follows the principle of least privilege and significantly reduces the security risk.

Comment on lines 51 to 62
- name: discovery.type
value: single-node
- name: ELASTIC_USERNAME
valueFrom:
secretKeyRef:
name: elasticsearch
key: username
- name: ELASTIC_PASSWORD
valueFrom:
secretKeyRef:
name: elasticsearch
key: password
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use a dedicated secret for Elasticsearch credentials

While using secrets for storing credentials is good practice, it's better to use a dedicated secret for Elasticsearch credentials to enhance security and maintain a clear separation of concerns.

Create a new secret specifically for Elasticsearch:

apiVersion: v1
kind: Secret
metadata:
  name: elasticsearch-credentials
type: Opaque
data:
  username: <base64-encoded-username>
  password: <base64-encoded-password>

Then update the env section to reference this new secret:

valueFrom:
  secretKeyRef:
-   name: elasticsearch
+   name: elasticsearch-credentials
    key: username

Make the same change for the password environment variable.

Comment on lines 113 to 138
# Elasticsearch Persistent Volume Claim
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
name: es-pvc
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 500Gi
storageClassName: do-block-storage-retain

---
# Elasticsearch Snapshot Repository Persistent Volume Claim
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
name: es-backup-pvc
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 500Gi
storageClassName: do-block-storage-retain
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using a storage class that supports resizing

The PersistentVolumeClaims are using the do-block-storage-retain storage class. While the retain policy is good for data persistence, it's worth considering if this storage class supports dynamic volume expansion.

Investigate if your storage provider offers a storage class with both retain policy and resizing capabilities. If available, update the PVCs to use such a storage class. This would allow for easier capacity expansion in the future without needing to migrate data.

For example, if a suitable storage class exists:

- storageClassName: do-block-storage-retain
+ storageClassName: do-block-storage-retain-resizable

Ensure to verify the exact name and capabilities of available storage classes in your environment.

Committable suggestion was skipped due to low confidence.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (5)
kube/indexer/swap/swap-txns-main-deployment.yaml (5)

20-20: Approve image update with suggestion for improved tagging.

The container image has been updated to a new SHA, which is good for deploying a specific version. However, consider adding a more descriptive tag (e.g., version number or feature name) alongside the SHA for better readability and easier rollback if needed.

Consider updating the image tag to include both a descriptive tag and the SHA:

image: starcoin/starcoin_indexer:v1.2.3-sha-1223fc3

29-29: Approve task expansion with documentation suggestion.

The BG_TASK_JOBS environment variable has been expanded to include additional tasks, which aligns with the PR objectives. This change enhances the indexer's functionality for swap-related operations.

Consider adding comments or documentation explaining the purpose of each new task (swap_transaction, swap_stats, swap_pool_fee_stat) to improve maintainability.


52-52: Approve PostgreSQL configuration change with optimization suggestion.

The database connection (DS_URL) has been updated to use a local Kubernetes PostgreSQL service, which is consistent with the move towards internal services. This change reduces external dependencies and potentially improves performance.

Consider the following optimization:

  1. Implement connection pooling to improve performance and resource utilization. This can be done by adding connection pool parameters to the JDBC URL or by using a connection pooling library like HikariCP.

Example with connection pool parameters:

- name: DS_URL
  value: "jdbc:postgresql://postgres-service.default.svc.cluster.local/starcoin?maxPoolSize=10&minIdle=5"

61-61: Approve consistent secret naming with suggestion.

The secret name for DB_PWD has been updated to 'postgresql', making it consistent with the secret used for DB_USER_NAME. This consistency is good for maintainability.

Consider using a more descriptive secret name that indicates its purpose, such as 'postgresql-credentials' or 'starcoin-db-credentials'. This would make it clearer what the secret contains while still maintaining consistency.

Example:

secretKeyRef:
  name: starcoin-db-credentials
  key: password

Line range hint 20-61: Overall changes improve security and align with project goals.

The changes in this file reflect a significant shift from using AWS managed services to internal Kubernetes services for both Elasticsearch and PostgreSQL. This move can potentially reduce costs and increase control over the infrastructure. The security improvements, such as using secrets for database credentials, are commendable.

The expansion of indexer tasks aligns well with the PR objectives of implementing custom indexing for swap data.

Consider the following recommendations:

  1. Monitor the performance of the Elasticsearch and PostgreSQL services after moving them in-cluster. Be prepared to adjust resource allocations or consider using node affinity rules to optimize their placement.

  2. Implement proper backup and disaster recovery procedures for the in-cluster databases, as you no longer have the automatic backups provided by AWS managed services.

  3. Set up monitoring and alerting for these critical services to ensure their health and performance.

  4. Consider using a service mesh like Istio for additional security features and traffic management between services.

  5. Regularly review and update the Kubernetes NetworkPolicies to ensure proper isolation and security of these services.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between b1633d2 and 942bf87.

📒 Files selected for processing (2)
  • kube/indexer/swap/swap-stat-main-deployment.yaml (1 hunks)
  • kube/indexer/swap/swap-txns-main-deployment.yaml (3 hunks)
🔇 Additional comments (8)
kube/indexer/swap/swap-txns-main-deployment.yaml (1)

54-57: Approve secure handling of database username.

The DB_USER_NAME is now set using a Kubernetes secret instead of a hardcoded value. This change significantly improves security by not exposing sensitive information in the configuration file.

kube/indexer/swap/swap-stat-main-deployment.yaml (7)

Line range hint 1-68: Summary of changes and recommendations

The changes in this file generally improve the security and potentially the performance of the deployment by:

  1. Adding new background tasks
  2. Moving from external AWS services to internal Kubernetes services
  3. Consistently using Kubernetes secrets for sensitive information

To ensure a smooth deployment, please verify:

  1. Resource allocation for the new background tasks
  2. Proper setup of Elasticsearch and PostgreSQL services within the cluster
  3. Existence and correct configuration of the 'elasticsearch' and 'postgresql' secrets
  4. Network policies to secure the Elasticsearch service, especially since it's now using HTTP

These changes are approved pending the suggested verifications.


64-65: Consistent use of Kubernetes secrets for PostgreSQL password.

The DB_PWD environment variable now references the 'postgresql' secret, which is consistent with the username configuration. This change maintains good security practices.

Please ensure that the 'postgresql' secret is properly set up in the cluster with both the 'username' and 'password' keys. Run the following script to verify the secret's contents:

#!/bin/bash
# Description: Verify the contents of the postgresql secret

# Test: Check for the postgresql secret definition and its keys
rg --json -g 'kube/**/*.yaml' 'kind:\s*Secret' -A 10 | jq -r 'select(.data.lines.text | contains("postgresql")) | .data.lines.text'

If the secret is not found or doesn't contain both required keys, make sure to update it before deploying this configuration.


57-60: Good use of Kubernetes secrets for PostgreSQL username.

The change to DB_USER_NAME to use a secret reference improves the security of the deployment. This is consistent with the best practices used for the Elasticsearch credentials.

Please ensure that the 'postgresql' secret is properly set up in the cluster with the required 'username' key. Run the following script to verify the secret's existence:

#!/bin/bash
# Description: Verify the existence of the postgresql secret

# Test: Check for the postgresql secret definition
rg --json -g 'kube/**/*.yaml' 'kind:\s*Secret' -A 5 | jq -r 'select(.data.lines.text | contains("postgresql")) | .data.lines.text'

If the secret is not found, make sure to create it before deploying this configuration.


37-39: Review security implications of using HTTP for Elasticsearch.

The STARCOIN_ES_PROTOCOL has been changed from HTTPS to HTTP, and the STARCOIN_ES_PORT from 443 to 9200. While this is likely fine for internal cluster communication, it's important to ensure that this doesn't introduce any security vulnerabilities.

Please verify that:

  1. The Elasticsearch service is not exposed outside the cluster.
  2. Appropriate network policies are in place to restrict access to the Elasticsearch service.

Run the following script to check for any NetworkPolicy related to Elasticsearch:

If no relevant NetworkPolicy is found, consider adding one to restrict access to the Elasticsearch service.


41-49: Excellent use of Kubernetes secrets for Elasticsearch credentials.

The changes to STARCOIN_ES_USER and the addition of STARCOIN_ES_PWD, both using secret references, significantly improve the security of the deployment. This is a best practice for handling sensitive information.

Please ensure that the 'elasticsearch' secret is properly set up in the cluster with the required 'username' and 'password' keys. Run the following script to verify the secret's existence:

If the secret is not found, make sure to create it before deploying this configuration.


55-55: Verify PostgreSQL service setup in the cluster.

The DS_URL has been updated to use an internal Kubernetes service URL for PostgreSQL. This is a good change that can improve performance and security.

Please ensure that the PostgreSQL service is properly set up in the cluster. Run the following script to verify the service:

If the service is not found, make sure to create it before deploying this configuration.


35-35: Verify Elasticsearch service setup in the cluster.

The STARCOIN_ES_URL has been updated to use an internal Kubernetes service URL. This is a good change that can improve performance and security.

Please ensure that the Elasticsearch service is properly set up in the cluster. Run the following script to verify the service:

If the service is not found, make sure to create it before deploying this configuration.

Comment on lines +40 to +44
value: "elasticsearch.default.svc.cluster.local"
- name: STARCOIN_ES_PROTOCOL
value: "https"
value: "http"
- name: STARCOIN_ES_PORT
value: "443"
value: "9200"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Approve Elasticsearch configuration change with security concerns.

The Elasticsearch connection has been updated to use a local Kubernetes service, which is good for reducing external dependencies. However, there are some security considerations to address:

  1. The protocol has been changed from HTTPS to HTTP, which could expose data in transit.
  2. The default Elasticsearch port (9200) is now being used, which might be more susceptible to unauthorized access attempts.

Consider the following security enhancements:

  1. Use HTTPS instead of HTTP for the Elasticsearch connection:
    - name: STARCOIN_ES_PROTOCOL
      value: "https"
  2. If possible, use a non-standard port for Elasticsearch to add an extra layer of security.
  3. Ensure that proper network policies are in place to restrict access to the Elasticsearch service within the cluster.

@@ -26,36 +26,42 @@ spec:
- name: NETWORK
value: "main.0727"
- name: BG_TASK_JOBS
value: "swap_stats"
value: "swap_stats,price_hour,price_stat"
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Add resource requests and limits for the container.

The swap_stat_main_deployment.yaml file does not specify resource requests and limits for the starcoin-indexer-stat container. Defining these ensures that the container has sufficient resources to handle the new background tasks (price_hour and price_stat) and prevents potential resource contention.

  • File: kube/indexer/swap/swap-stat-main-deployment.yaml
  • Container: starcoin-indexer-stat

Please add appropriate resources specifications to ensure optimal performance and reliability.

🔗 Analysis chain

Verify resource allocation for new background tasks.

The BG_TASK_JOBS environment variable has been updated to include two new tasks: price_hour and price_stat. This is a good addition for expanding functionality.

However, please ensure that the container has sufficient resources allocated to handle these additional tasks. Run the following script to check the current resource allocation:

If necessary, consider updating the resource requests and limits to accommodate the increased workload.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check resource allocation for the starcoin-indexer-stat container

# Test: Extract and display resource requests and limits
rg --json -g 'kube/indexer/swap/swap-stat-main-deployment.yaml' 'resources:' -A 10 | jq -r '.data.lines.text'

Length of output: 110

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (5)
kube/subscribe/starscan-sub-main-deployment.yaml (5)

31-31: Approved: Good move to use internal Elasticsearch service.

Switching from an external AWS Elasticsearch endpoint to a cluster-local service is a positive change. It reduces external dependencies and potential security risks.

Consider using an environment variable for the namespace (currently hardcoded as "default") to improve flexibility:

value: "elasticsearch.$(NAMESPACE).svc.cluster.local"

Then, set the NAMESPACE environment variable in the deployment configuration.


37-40: Approved: Good security practice using secrets for Elasticsearch username.

Switching from a hardcoded Elasticsearch username to a secret reference is an excellent security improvement. This aligns with Kubernetes best practices for handling sensitive information.

For consistency, consider using a single secret for all Elasticsearch-related credentials. You could structure your secret like this:

apiVersion: v1
kind: Secret
metadata:
  name: elasticsearch-credentials
type: Opaque
data:
  username: <base64-encoded-username>
  password: <base64-encoded-password>

Then update the reference in the deployment:

valueFrom:
  secretKeyRef:
    name: elasticsearch-credentials
    key: username

This approach centralizes all Elasticsearch credentials in one secret, making it easier to manage.


41-45: Approved: Excellent addition of secret for Elasticsearch password.

Adding a secret reference for the Elasticsearch password significantly enhances the security of the deployment. This is a crucial improvement in handling sensitive information.

As mentioned in the previous comment, consider unifying all Elasticsearch credentials into a single secret for better manageability. If you implement this suggestion, you would update this section to:

- name: STARCOIN_ES_PWD
  valueFrom:
    secretKeyRef:
      name: elasticsearch-credentials
      key: password

This approach maintains consistency across all Elasticsearch-related secret references.


51-51: Approved: Good move to use internal PostgreSQL service.

Switching from an external AWS RDS instance to a cluster-local PostgreSQL service is a positive change. It reduces external dependencies and potential security risks, aligning with best practices for Kubernetes deployments.

To improve flexibility, consider parameterizing the database URL:

value: "jdbc:postgresql://$(DB_HOST)/$(DB_NAME)"

Then, add environment variables for DB_HOST and DB_NAME:

- name: DB_HOST
  value: "postgres-service.default.svc.cluster.local"
- name: DB_NAME
  value: "starcoin"

This approach makes it easier to change the database configuration in the future without modifying the main connection string.


53-61: Approved: Excellent use of secrets for database credentials.

Switching to secret references for both DB_USER_NAME and DB_PWD is a crucial security improvement. This change aligns perfectly with Kubernetes best practices for handling sensitive information.

To further improve secret management, consider:

  1. Using a consistent naming convention across all secrets in your cluster.
  2. Implementing a secret rotation strategy.

For example, you could name your secret more specifically:

secretKeyRef:
  name: starscan-postgresql-credentials
  key: username

And for the password:

secretKeyRef:
  name: starscan-postgresql-credentials
  key: password

This naming convention makes it clear which application and service the secret is for, improving overall secret management in your cluster.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 942bf87 and 0df6e41.

📒 Files selected for processing (2)
  • kube/indexer/swap/swap-txns-main-deployment.yaml (2 hunks)
  • kube/subscribe/starscan-sub-main-deployment.yaml (1 hunks)
🔇 Additional comments (9)
kube/subscribe/starscan-sub-main-deployment.yaml (3)

Line range hint 1-61: Overall assessment: Positive changes with room for further improvements.

The changes in this deployment configuration represent a significant step towards better security practices and more Kubernetes-native configurations. Key improvements include:

  1. Moving to internal cluster services for Elasticsearch and PostgreSQL.
  2. Implementing secret references for sensitive information.

To further enhance this configuration, consider the following recommendations:

  1. Unify and standardize secret management across the deployment.
  2. Parameterize configuration values where possible to improve flexibility.
  3. Review and potentially reinstate HTTPS for Elasticsearch communication.
  4. Implement a secret rotation strategy for long-term security management.

These changes have significantly improved the deployment's security posture. Implementing the suggested refinements will further enhance the configuration's robustness and maintainability.


35-35: Port change is consistent with protocol switch, but consider secure alternatives.

The change to port 9200 is consistent with the switch to HTTP protocol for Elasticsearch. However, this should be reviewed in conjunction with the previous comment about keeping HTTPS.

If you decide to keep HTTPS (recommended):

  1. Change the port back to 443 or use another secure port (e.g., 9243 for HTTPS).
  2. Ensure the Elasticsearch service is configured to listen on this secure port.

You can verify the current Elasticsearch port configuration with:

#!/bin/bash
# Check Elasticsearch port configuration
kubectl get svc elasticsearch -o=jsonpath='{.spec.ports[0].port}'

Ensure this matches the port specified in the STARCOIN_ES_PORT environment variable.


33-33: ⚠️ Potential issue

Consider keeping HTTPS for Elasticsearch communication.

Switching from HTTPS to HTTP removes encryption for Elasticsearch communication. While this might be acceptable for internal cluster communication, it's generally recommended to use HTTPS even for internal services to maintain a consistent security posture.

Consider keeping HTTPS and ensure that the Elasticsearch service is configured to use TLS. You may need to:

  1. Set up a TLS certificate for the Elasticsearch service.
  2. Configure Elasticsearch to use HTTPS.
  3. Update the STARCOIN_ES_PROTOCOL back to "https".
  4. Ensure that the application trusts the Elasticsearch certificate.

To verify the current Elasticsearch configuration, you can run:

If this command succeeds, Elasticsearch is already configured for HTTPS, and you should keep the protocol as "https".

kube/indexer/swap/swap-txns-main-deployment.yaml (6)

20-20: Approve container image update with verification suggestion.

The container image has been updated to a newer version, which is good for incorporating the latest features and fixes. Using a specific SHA for versioning ensures reproducibility.

To ensure the new image is compatible and functioning as expected, please verify that:

  1. The new image has been tested in a staging environment.
  2. Any new features or changes in the image are documented and align with the project requirements.
  3. There are no breaking changes that could affect the application's functionality.

29-29: Approve BG_TASK_JOBS update with resource consideration.

The expansion of background tasks to include swap-related operations is a good enhancement to the indexer's functionality.

Please ensure that:

  1. The system has sufficient resources (CPU, memory) to handle these additional tasks without performance degradation.
  2. The new tasks have been tested thoroughly in a staging environment.
  3. Monitoring and alerting systems are updated to cover these new tasks.

Consider running a load test to verify the system's performance with these additional tasks.


35-39: Acknowledge Elasticsearch configuration change with security reminder.

The Elasticsearch connection has been updated to use a local Kubernetes service, which can improve performance and reduce external dependencies. However, please refer to the existing comment regarding security concerns about using HTTP instead of HTTPS and the default Elasticsearch port.


41-49: Approve use of secret references for Elasticsearch credentials.

The transition to using secret references for Elasticsearch username and password is a significant security improvement. This practice helps protect sensitive information and allows for easier credential rotation.

Please ensure that:

  1. The referenced secret elasticsearch exists in the cluster and contains the correct username and password keys.
  2. The secret is properly managed and rotated as part of your security practices.
  3. Access to this secret is restricted to only the necessary service accounts or users.

55-55: Approve database URL update with verification steps.

The transition from an AWS RDS instance to a local PostgreSQL service is consistent with the move towards internal services. This can potentially improve performance and reduce external dependencies.

Please ensure that:

  1. The postgres-service is correctly set up and running in the default namespace.
  2. The service has the necessary resources and configuration to handle the expected load.
  3. Data migration from the AWS RDS instance to the local PostgreSQL service has been performed correctly, if applicable.
  4. Backup and disaster recovery procedures have been updated to account for this change.
  5. The application's database connection pool settings are optimized for the new setup.

57-60: Approve use of secret references for database credentials.

The transition to using secret references for both the database username and password enhances the security of the deployment. The change in the secret name for the password suggests a reorganization of secrets, which can improve secret management.

Please ensure that:

  1. The referenced secret postgresql exists in the cluster and contains the correct username and password keys.
  2. The secret is properly managed and rotated as part of your security practices.
  3. Access to this secret is restricted to only the necessary service accounts or users.
  4. If the secret reorganization affects other deployments, ensure they are updated accordingly.
  5. Consider using a secrets management solution (e.g., HashiCorp Vault, AWS Secrets Manager) for centralized management of secrets across your infrastructure.

Also applies to: 64-65

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Outside diff range and nitpick comments (4)
kube/elasticsearch/elasticsearch-pv-snapshot.yaml (1)

1-21: Consider implementing a snapshot scheduling strategy

While the snapshot configuration is correct, consider implementing a CronJob to schedule regular snapshots. This ensures automated backup of your Elasticsearch data.

Would you like me to provide an example CronJob configuration for scheduling regular snapshots?

🧰 Tools
🪛 yamllint (1.35.1)

[error] 21-21: no new line character at the end of file

(new-line-at-end-of-file)

kube/elasticsearch/elasticsearch-storage.yaml (1)

15-26: Consider different storage classes for backup PVC

Both PVCs use the same storage class. Consider using a different storage class for backups that might be optimized for:

  1. Cost-effectiveness for backup storage
  2. Different performance characteristics
  3. Different availability zones for disaster recovery
🧰 Tools
🪛 yamllint (1.35.1)

[error] 26-26: no new line character at the end of file

(new-line-at-end-of-file)

kube/elasticsearch/elasticsearch-deployment.yaml (2)

7-7: Consider high availability configuration

Single replica deployment may lead to service interruption during updates or node failures.

Consider:

  1. Increasing replicas for high availability
  2. Adding pod disruption budget
  3. Implementing proper anti-affinity rules
spec:
  replicas: 3
  podAntiAffinity:
    requiredDuringSchedulingIgnoredDuringExecution:
    - labelSelector:
        matchExpressions:
        - key: app
          operator: In
          values:
          - elasticsearch
      topologyKey: "kubernetes.io/hostname"

28-34: Review resource allocation

The current resource configuration might need adjustment:

  • Memory limit (6Gi) is only 1.5x the request (4Gi)
  • CPU limit might cause throttling

Consider:

  1. Setting memory limit to 2x-3x the request for Elasticsearch
  2. Removing CPU limits to prevent throttling
  3. Adjusting based on actual usage metrics
resources:
  requests:
    cpu: "1"
    memory: "4Gi"
  limits:
    memory: "12Gi"
    # Remove CPU limit to prevent throttling
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 0df6e41 and c14e367.

📒 Files selected for processing (6)
  • kube/elasticsearch/elasticsearch-config.yaml (1 hunks)
  • kube/elasticsearch/elasticsearch-deployment.yaml (1 hunks)
  • kube/elasticsearch/elasticsearch-pv-snapshot.yaml (1 hunks)
  • kube/elasticsearch/elasticsearch-service.yaml (1 hunks)
  • kube/elasticsearch/elasticsearch-storage-snapshot.yaml (1 hunks)
  • kube/elasticsearch/elasticsearch-storage.yaml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • kube/elasticsearch/elasticsearch-service.yaml
🧰 Additional context used
🪛 yamllint (1.35.1)
kube/elasticsearch/elasticsearch-config.yaml

[error] 16-16: no new line character at the end of file

(new-line-at-end-of-file)

kube/elasticsearch/elasticsearch-storage.yaml

[error] 26-26: no new line character at the end of file

(new-line-at-end-of-file)

kube/elasticsearch/elasticsearch-pv-snapshot.yaml

[error] 21-21: no new line character at the end of file

(new-line-at-end-of-file)

kube/elasticsearch/elasticsearch-storage-snapshot.yaml

[error] 28-28: no new line character at the end of file

(new-line-at-end-of-file)

kube/elasticsearch/elasticsearch-deployment.yaml

[warning] 77-77: wrong indentation: expected 12 but found 14

(indentation)


[error] 93-93: no new line character at the end of file

(new-line-at-end-of-file)

🪛 checkov (3.2.312)
kube/elasticsearch/elasticsearch-deployment.yaml

[HIGH] 2-91: Container should not be privileged

(CKV_K8S_16)

🔇 Additional comments (2)
kube/elasticsearch/elasticsearch-storage.yaml (1)

1-12: Review storage requirements and implement resource quotas

The PVC requests a significant amount of storage (500Gi). Consider:

  1. Implementing namespace resource quotas to prevent uncontrolled storage allocation
  2. Documenting storage sizing requirements
  3. Setting up storage monitoring and alerts
kube/elasticsearch/elasticsearch-storage-snapshot.yaml (1)

7-8: Verify storage capacity requirements

The storage size of 1Ti seems quite large for snapshot storage. Please verify:

  1. Expected snapshot size and frequency
  2. Retention policy
  3. Cleanup strategy

Also applies to: 26-27

✅ Verification successful

Storage capacity aligns with other Elasticsearch volumes

Based on the search results, the 1Ti storage for snapshots is reasonable considering:

  • The main Elasticsearch storage volumes are configured with 500Gi each
  • This is a blockchain data indexer storing historical transaction data
  • Snapshots need to accommodate potential growth and multiple backup points

The larger snapshot volume (1Ti vs 500Gi) provides appropriate headroom for:

  • Multiple point-in-time snapshots
  • Growth of the indexed blockchain data
  • Preventing out-of-space issues during snapshot operations
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are any other PVs/PVCs in the cluster to compare sizes
kubectl get pv,pvc -A -o custom-columns=NAME:.metadata.name,SIZE:.spec.capacity.storage

Length of output: 187


Script:

#!/bin/bash
# Let's check the repository for any documentation about storage requirements
fd README -t f -X cat {}

# Also search for any configuration files that might have storage-related settings
rg -i "storage|capacity|retention|cleanup" --type yaml

Length of output: 4978

Comment on lines +16 to +21
metadata:
name: es-pv-snapshot
spec:
volumeSnapshotClassName: do-snapshot-class
source:
persistentVolumeClaimName: es-pvc
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Specify namespace for the VolumeSnapshot resource

The VolumeSnapshot resource should explicitly specify a namespace to ensure it's created in the correct namespace where the PVC exists.

 kind: VolumeSnapshot
 metadata:
   name: es-pv-snapshot
+  namespace: default  # Replace with your target namespace
 spec:
   volumeSnapshotClassName: do-snapshot-class
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
metadata:
name: es-pv-snapshot
spec:
volumeSnapshotClassName: do-snapshot-class
source:
persistentVolumeClaimName: es-pvc
metadata:
name: es-pv-snapshot
namespace: default # Replace with your target namespace
spec:
volumeSnapshotClassName: do-snapshot-class
source:
persistentVolumeClaimName: es-pvc
🧰 Tools
🪛 yamllint (1.35.1)

[error] 21-21: no new line character at the end of file

(new-line-at-end-of-file)

Comment on lines 7 to 16
elasticsearch.yml: |
xpack.security.enabled: true
xpack.license.self_generated.type: basic
network.host: 0.0.0.0
path.repo: ["/data/es_snapshot_repository"]
s3.client.default.endpoint: "s3.ap-northeast-1.amazonaws.com"
s3.client.default.protocol: https
s3.client.default.read_timeout: 50s
s3.client.default.max_retries: 3
s3.client.default.use_throttle_retries: true
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add essential Elasticsearch configurations

The current configuration is missing several important settings:

  1. JVM heap size configuration
  2. Node roles definition
  3. Cluster settings (discovery, minimum_master_nodes)
  4. Resource limits

Would you like me to provide a more comprehensive configuration that includes these essential settings?

🧰 Tools
🪛 yamllint (1.35.1)

[error] 16-16: no new line character at the end of file

(new-line-at-end-of-file)

Comment on lines +8 to +10
xpack.security.enabled: true
xpack.license.self_generated.type: basic
network.host: 0.0.0.0
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Security configuration requires additional measures

The security configuration needs enhancement:

  • While xpack.security is enabled, the current setup lacks TLS/SSL configuration
  • The service exposes port 9200 without any transport encryption
  • AWS credentials are properly managed through secrets, but Elasticsearch authentication isn't configured
  • The network.host: 0.0.0.0 setting combined with the exposed service creates unnecessary security risk

Recommendations:

  • Add TLS/SSL configuration for transport encryption
  • Configure authentication for Elasticsearch access
  • Consider restricting network.host to specific interfaces if possible
  • Review service configuration to limit access to necessary components only
🔗 Analysis chain

Review security configuration

While xpack security is enabled, there are several security considerations to address:

  1. Setting network.host: 0.0.0.0 exposes Elasticsearch to all network interfaces
  2. Authentication configuration is not visible
  3. Consider implementing TLS for secure communication
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any TLS/security related configurations in other yaml files
rg -l "xpack.security" 
rg -l "ssl.enabled"

Length of output: 87


Script:

#!/bin/bash
# Let's check for any other security-related configurations in yaml files
rg -l "tls\." kube/
rg -l "ssl\." kube/
rg -l "xpack\.security" -A 5 kube/

# Check for authentication configurations
rg -l "auth" kube/elasticsearch/
rg -l "password" kube/elasticsearch/
rg -l "credentials" kube/elasticsearch/

# Check for other elasticsearch configuration files
fd . kube/ -e yaml -e yml | grep -i elastic

Length of output: 670


Script:

#!/bin/bash
# Let's check the content of elasticsearch-deployment.yaml for password/credential configurations
rg "password|credential" -A 3 kube/elasticsearch/elasticsearch-deployment.yaml

# Check the service configuration to understand network exposure
cat kube/elasticsearch/elasticsearch-service.yaml

# Look for any security-related environment variables or secrets
rg "valueFrom|secretKeyRef|configMapKeyRef" kube/elasticsearch/elasticsearch-deployment.yaml

Length of output: 1004

Comment on lines +12 to +14
storageClassName: do-block-storage-retain
hostPath:
path: /data/elasticsearch-snapshots
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security concern: hostPath usage in production

Using hostPath in production environments poses several risks:

  • Data persistence depends on the specific node
  • No data replication or backup
  • Security implications as it grants access to host filesystem

Consider using a more production-ready storage solution:

  1. Cloud provider's managed storage (e.g., AWS EBS, Azure Disk)
  2. Network storage solutions (e.g., NFS)
  3. Distributed storage systems (e.g., Rook-Ceph)

Also, if hostPath must be used, add nodeAffinity to ensure the pod runs on the correct node:

  nodeAffinity:
    required:
      nodeSelectorTerms:
      - matchExpressions:
        - key: kubernetes.io/hostname
          operator: In
          values:
          - your-specific-node

Comment on lines +25 to +26
securityContext:
privileged: true
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Remove privileged container security context

Running containers with privileged access is a significant security risk as it grants extensive host-level permissions.

Replace the privileged security context with minimal required permissions:

securityContext:
  runAsUser: 1000
  runAsGroup: 1000
  fsGroup: 1000
  capabilities:
    drop:
      - ALL

Comment on lines 63 to 75
# S3 配置
- name: S3_CLIENT_DEFAULT_REGION
value: "ap-northeast-1"
- name: S3_CLIENT_ACCESS_KEY
valueFrom:
secretKeyRef:
name: aws-credentials
key: access-key
- name: S3_CLIENT_SECRET_KEY
valueFrom:
secretKeyRef:
name: aws-credentials
key: secret-key
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Security: Improve sensitive credentials handling

The current AWS credentials configuration could be improved for better security.

Consider:

  1. Using IAM roles instead of static credentials where possible
  2. Implementing proper secret rotation
  3. Using external secret management solutions (e.g., HashiCorp Vault)

Example using AWS IRSA (IAM Roles for Service Accounts):

serviceAccountName: elasticsearch-sa
# Remove the environment variables section for AWS credentials
# Instead, annotate the ServiceAccount:
---
apiVersion: v1
kind: ServiceAccount
metadata:
  name: elasticsearch-sa
  annotations:
    eks.amazonaws.com/role-arn: arn:aws:iam::ACCOUNT_ID:role/elasticsearch-s3-role

Comment on lines 79 to 81
command: ["/bin/bash", "-c", "/usr/share/elasticsearch/bin/elasticsearch-plugin list | grep -q repository-s3 || /usr/share/elasticsearch/bin/elasticsearch-plugin install --batch repository-s3 && \
echo ${S3_CLIENT_ACCESS_KEY} | /usr/share/elasticsearch/bin/elasticsearch-keystore add s3.client.default.access_key --stdin &&\
echo ${S3_CLIENT_SECRET_KEY} | /usr/share/elasticsearch/bin/elasticsearch-keystore add s3.client.default.secret_key --stdin"]
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve lifecycle hook reliability

The current postStart hook implementation has several potential issues:

  1. No error handling
  2. No verification of successful plugin installation
  3. No timeout mechanism

Consider moving this logic to an init container for better reliability:

initContainers:
- name: setup-plugins
  image: docker.elastic.co/elasticsearch/elasticsearch:7.17.0
  command:
  - /bin/bash
  - -c
  - |
    set -e
    if ! /usr/share/elasticsearch/bin/elasticsearch-plugin list | grep -q repository-s3; then
      /usr/share/elasticsearch/bin/elasticsearch-plugin install --batch repository-s3
    fi
    echo "${S3_CLIENT_ACCESS_KEY}" | /usr/share/elasticsearch/bin/elasticsearch-keystore add s3.client.default.access_key --stdin
    echo "${S3_CLIENT_SECRET_KEY}" | /usr/share/elasticsearch/bin/elasticsearch-keystore add s3.client.default.secret_key --stdin
    # Verify keystore entries
    /usr/share/elasticsearch/bin/elasticsearch-keystore list | grep -q s3.client.default.access_key
    /usr/share/elasticsearch/bin/elasticsearch-keystore list | grep -q s3.client.default.secret_key

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (2)
kube/elasticsearch/elasticsearch-deployment.yaml (2)

7-14: Consider production-ready configurations

For production environments, consider:

  1. Increasing the number of replicas for high availability
  2. Adding pod anti-affinity rules to ensure replicas are scheduled on different nodes
spec:
  replicas: 3  # Adjust based on your needs
  template:
    spec:
      affinity:
        podAntiAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            - labelSelector:
                matchExpressions:
                  - key: app
                    operator: In
                    values:
                      - elasticsearch
              topologyKey: "kubernetes.io/hostname"

73-74: Remove commented code and add newline

Clean up the configuration by:

  1. Removing the commented s3-mount-point volume if it's not needed
  2. Adding a newline at the end of the file
🧰 Tools
🪛 yamllint (1.35.1)

[error] 74-74: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between c14e367 and 20fa7fa.

📒 Files selected for processing (1)
  • kube/elasticsearch/elasticsearch-deployment.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
kube/elasticsearch/elasticsearch-deployment.yaml

[error] 74-74: no new line character at the end of file

(new-line-at-end-of-file)

🪛 checkov (3.2.312)
kube/elasticsearch/elasticsearch-deployment.yaml

[HIGH] 2-72: Container should not be privileged

(CKV_K8S_16)

🔇 Additional comments (2)
kube/elasticsearch/elasticsearch-deployment.yaml (2)

25-26: ⚠️ Potential issue

Critical: Remove privileged container security context

Running containers with privileged access is a significant security risk as it grants extensive host-level permissions.


64-72: Verify referenced resources exist

Ensure that the following resources are properly defined:

  • PVC: es-pvc
  • PVC: es-backup-pvc
  • ConfigMap: elasticsearch-config
✅ Verification successful

All referenced resources are properly defined

The verification confirms that all required resources exist in the repository:

  • PVC es-pvc is defined in elasticsearch-storage.yaml
  • PVC es-backup-pvc is defined in elasticsearch-storage.yaml
  • ConfigMap elasticsearch-config is defined in elasticsearch-config.yaml
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for referenced resources in the kubernetes configs
echo "Checking for PVC definitions..."
rg -l "kind:\s+PersistentVolumeClaim" kube/
rg -l "claimName:\s+(es-pvc|es-backup-pvc)" kube/

echo "Checking for ConfigMap definition..."
rg -l "kind:\s+ConfigMap.*name:\s+elasticsearch-config" kube/

Length of output: 515


Script:

#!/bin/bash
# Let's check the actual content of the PVC and ConfigMap definitions
echo "Checking PVC definitions in elasticsearch-storage.yaml..."
cat kube/elasticsearch/elasticsearch-storage.yaml

echo -e "\nChecking PVC definitions in elasticsearch-storage-snapshot.yaml..."
cat kube/elasticsearch/elasticsearch-storage-snapshot.yaml

echo -e "\nChecking for elasticsearch-config ConfigMap..."
rg -l "kind:\s*ConfigMap.*name:\s*elasticsearch-config" kube/
rg "kind:\s*ConfigMap.*name:\s*elasticsearch-config" -A 5 kube/

Length of output: 1694


Script:

#!/bin/bash
# Let's check for the elasticsearch ConfigMap definition in a different way
echo "Searching for elasticsearch ConfigMap..."
rg -l "kind:\s*ConfigMap" kube/elasticsearch/
cat kube/elasticsearch/elasticsearch-config*.yaml || echo "No elasticsearch config file found"

Length of output: 745

🧰 Tools
🪛 checkov (3.2.312)

[HIGH] 2-72: Container should not be privileged

(CKV_K8S_16)

Comment on lines +16 to +22
initContainers:
- name: init-permissions
image: busybox
command: [ "sh", "-c", "chown -R 1000:1000 /usr/share/elasticsearch/data" ]
volumeMounts:
- name: es-data
mountPath: /usr/share/elasticsearch/data
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add security context to init container

While the init container correctly sets permissions, it should run with a defined security context.

initContainers:
  - name: init-permissions
    image: busybox
    securityContext:
      runAsUser: 0  # Temporarily run as root for chown
      runAsNonRoot: false
      allowPrivilegeEscalation: false
      capabilities:
        drop:
          - ALL
    command: ["sh", "-c", "chown -R 1000:1000 /usr/share/elasticsearch/data"]

Comment on lines +49 to +61
env:
- name: discovery.type
value: single-node
- name: ELASTIC_USERNAME
valueFrom:
secretKeyRef:
name: elasticsearch
key: username
- name: ELASTIC_PASSWORD
valueFrom:
secretKeyRef:
name: elasticsearch
key: password
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add health probes for better container lifecycle management

The deployment lacks readiness and liveness probes, which are crucial for proper container lifecycle management.

containers:
  - name: elasticsearch
    # ... existing config ...
    livenessProbe:
      httpGet:
        path: /_cluster/health
        port: 9200
        scheme: HTTP
      initialDelaySeconds: 300
      periodSeconds: 30
    readinessProbe:
      httpGet:
        path: /_cluster/health?local=true
        port: 9200
        scheme: HTTP
      initialDelaySeconds: 60
      periodSeconds: 10

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
kube/elasticsearch/elasticsearch-config.yaml (1)

12-16: Document S3 integration plans

The commented S3 client settings suggest future integration plans. Consider:

  1. Adding documentation about when and how these settings will be used
  2. Creating a separate ConfigMap for S3 settings when needed
  3. Adding a newline at the end of file
     #    s3.client.default.protocol: https
     #    s3.client.default.read_timeout: 50s
     #    s3.client.default.max_retries: 3
-    #    s3.client.default.use_throttle_retries: true
+    #    s3.client.default.use_throttle_retries: true
+
🧰 Tools
🪛 yamllint (1.35.1)

[error] 16-16: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 20fa7fa and 32a86c4.

📒 Files selected for processing (1)
  • kube/elasticsearch/elasticsearch-config.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
kube/elasticsearch/elasticsearch-config.yaml

[error] 16-16: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (4)
kube/elasticsearch/elasticsearch-config.yaml (4)

1-7: LGTM! ConfigMap structure is well-defined.

The ConfigMap is properly structured with appropriate metadata.


8-9: ⚠️ Potential issue

Security configuration requires enhancement

While xpack security is enabled, critical security configurations are missing:

  • Authentication settings
  • TLS/SSL configuration
  • Node roles definition

Would you like me to provide a comprehensive security configuration example?


10-10: ⚠️ Potential issue

Restrict network host binding

Setting network.host: 0.0.0.0 exposes Elasticsearch to all network interfaces, which is a security risk. Consider restricting this to specific interfaces or using internal Kubernetes DNS.

-    network.host: 0.0.0.0
+    network.host: ${POD_IP}
+    discovery.seed_hosts: ["elasticsearch-service"]

Likely invalid or redundant comment.


11-11: Verify snapshot repository path

Ensure that the snapshot repository path /data/es_snapshot_repository is:

  1. Properly mounted in the pod
  2. Has appropriate permissions
✅ Verification successful

Let me gather more information about the volume configurations to ensure proper mounting and permissions.


Let me check the PVC and volume mount configurations in the elasticsearch deployment.


Snapshot repository path is properly configured

The snapshot repository path /data/es_snapshot_repository is correctly configured:

  1. It's properly mounted in the elasticsearch pod through the es-backup volume mount
  2. The permissions are handled appropriately:
    • The path is mounted with a PVC (es-backup-pvc)
    • The container runs with privileged security context
    • The elasticsearch process runs as user 1000:1000 (standard elasticsearch user)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for volume mounts and persistence configurations
rg -A 5 "/data/es_snapshot_repository" kube/
fd . kube/ -e yaml -e yml | xargs rg "volumeMounts|persistentVolumeClaim"

Length of output: 1836


Script:

#!/bin/bash
# Check the PV and PVC configurations for the snapshot repository
cat kube/elasticsearch/elasticsearch-pv-snapshot.yaml
rg -A 10 "kind: PersistentVolumeClaim.*es-pvc" kube/

Length of output: 575


Script:

#!/bin/bash
# Check the elasticsearch deployment configuration for volume mounts and PVC
cat kube/elasticsearch/elasticsearch-deployment.yaml

Length of output: 2166

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (1)
kube/indexer/stc-scan/stcscan-main-deployment.yaml (1)

1-17: Add production-ready configurations for high availability and resource management

Consider adding the following essential configurations for a production environment:

  • Increase replicas for high availability
  • Define resource limits and requests
  • Specify update strategy
  • Add pod disruption budget

Example additions:

spec:
  replicas: 2  # At least 2 for HA
  strategy:
    type: RollingUpdate
    rollingUpdate:
      maxSurge: 1
      maxUnavailable: 0
  template:
    spec:
      resources:
        requests:
          memory: "1Gi"
          cpu: "500m"
        limits:
          memory: "2Gi"
          cpu: "1000m"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 32a86c4 and 9716ad3.

📒 Files selected for processing (1)
  • kube/indexer/stc-scan/stcscan-main-deployment.yaml (1 hunks)
🔇 Additional comments (2)
kube/indexer/stc-scan/stcscan-main-deployment.yaml (2)

28-29: Verify background task configuration

The BG_TASK_JOBS environment variable contains multiple critical jobs. Please verify:

  • Job dependencies and execution order
  • Resource requirements for each job
  • Potential race conditions
✅ Verification successful

Based on the shell script results, I can now generate a final response since I have enough information about the job implementations and scheduling:

Background tasks are properly configured and scheduled

The jobs in BG_TASK_JOBS are well-defined in the codebase with clear responsibilities:

  • indexer: Main indexing job running every 10 seconds (IndexerHandleJob)
  • secondary: Secondary indexing for transfers running every 10 seconds (SecondaryIndexer)
  • market_cap: Market cap updates running daily (MarketCapIndexer)
  • txn_payload: Transaction payload processing every 10 seconds (TransactionPayloadHandle)
  • txn_global_idx_update: Global transaction index updates every 2 seconds (TransactionInfoIndexer)

The jobs are independently scheduled via Quartz with appropriate intervals and no direct dependencies between them. Resource isolation is handled by the Quartz scheduler.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for job implementations and dependencies
ast-grep --pattern 'class $_ implements $_Task {
  $$$
}'

# Look for job scheduling logic
rg -A 10 "indexer|secondary|market_cap|txn_payload|txn_global_idx_update"

Length of output: 76580


50-53: Review external API configuration

The Swap API configuration has potential issues:

  • Hardcoded URL without version path
  • No API version pinning
  • No retry/timeout configuration

Comment on lines +18 to +22
containers:
- name: starcoin-indexer-stcscan
image: starcoin/starcoin_indexer:sha-1223fc3
ports:
- containerPort: 8300
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance container reliability and security

The container configuration needs essential production safeguards:

  • Add liveness and readiness probes
  • Define security context
  • Specify image pull policy

Example additions:

containers:
  - name: starcoin-indexer-stcscan
    image: starcoin/starcoin_indexer:sha-1223fc3
    imagePullPolicy: IfNotPresent
    securityContext:
      runAsNonRoot: true
      runAsUser: 1000
      readOnlyRootFilesystem: true
    livenessProbe:
      httpGet:
        path: /health
        port: 8300
      initialDelaySeconds: 30
      periodSeconds: 10
    readinessProbe:
      httpGet:
        path: /ready
        port: 8300
      initialDelaySeconds: 5
      periodSeconds: 5

Comment on lines +34 to +39
- name: STARCOIN_ES_URL
value: "elasticsearch.default.svc.cluster.local"
- name: STARCOIN_ES_PROTOCOL
value: "http"
- name: STARCOIN_ES_PORT
value: "9200"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Secure Elasticsearch communication

The current configuration uses unsecured HTTP communication with Elasticsearch. Consider:

  • Enabling HTTPS
  • Adding TLS verification
- value: "http"
+ value: "https"

Also, add these environment variables:

- name: STARCOIN_ES_SSL_VERIFY
  value: "true"
- name: STARCOIN_ES_SSL_CA_CERT
  valueFrom:
    secretKeyRef:
      name: elasticsearch-certs
      key: ca.crt

Comment on lines +54 to +55
- name: DS_URL
value: "jdbc:postgresql://postgres-service.default.svc.cluster.local/starcoin"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Secure PostgreSQL connection

The JDBC URL should include SSL parameters for secure communication.

- value: "jdbc:postgresql://postgres-service.default.svc.cluster.local/starcoin"
+ value: "jdbc:postgresql://postgres-service.default.svc.cluster.local/starcoin?ssl=true&sslmode=verify-full"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: DS_URL
value: "jdbc:postgresql://postgres-service.default.svc.cluster.local/starcoin"
- name: DS_URL
value: "jdbc:postgresql://postgres-service.default.svc.cluster.local/starcoin?ssl=true&sslmode=verify-full"

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.

5 participants