-
Notifications
You must be signed in to change notification settings - Fork 33
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
add aws signer docs #2795
add aws signer docs #2795
Conversation
Warning Rate limit exceeded@trajan0x has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 19 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes introduce a detailed setup and configuration for the signer tool, migration of example references to a newly structured contract tests directory, and Docker build support enhancements. The signer documentation now includes AWS KMS configuration, while Go build configurations and Dockerfile enhancements facilitate deployment. Package imports and references are updated for clarity and organization. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2795 +/- ##
===================================================
- Coverage 25.91883% 25.31441% -0.60443%
===================================================
Files 739 715 -24
Lines 53274 52002 -1272
Branches 80 80
===================================================
- Hits 13808 13164 -644
+ Misses 38044 37514 -530
+ Partials 1422 1324 -98
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this 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.yaml
Review profile: CHILL
Files ignored due to path filters (16)
docs/bridge/docs/Services/img/create-acces-key.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/create-access-key.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/iam-dash.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/iam-preview-user.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/key-details.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/kms-1.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/kms-2.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/kms-advanced.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/kms-labels.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/kms-permissons.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/kms-user-permissions.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/kms-user.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/perms.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/review.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/user-list.png
is excluded by!**/*.png
docs/bridge/docs/Services/img/user-perms.png
is excluded by!**/*.png
Files selected for processing (24)
- docker/signer-example.Dockerfile (1 hunks)
- docs/bridge/docs/Services/Signer.md (1 hunks)
- ethergo/.goreleaser.yml (1 hunks)
- ethergo/README.md (2 hunks)
- ethergo/backends/anvil/suite_test.go (3 hunks)
- ethergo/deployer/deployed_contract_test.go (2 hunks)
- ethergo/examples/contracttests/README.md (3 hunks)
- ethergo/examples/contracttests/contracttype.go (1 hunks)
- ethergo/examples/contracttests/contracttypeimpl_string.go (1 hunks)
- ethergo/examples/contracttests/counter_test.go (3 hunks)
- ethergo/examples/contracttests/deployer.go (3 hunks)
- ethergo/examples/contracttests/deploymanager.go (1 hunks)
- ethergo/examples/contracttests/doc.go (1 hunks)
- ethergo/examples/signer-example/README.md (1 hunks)
- ethergo/examples/signer-example/cmd/cmd.go (1 hunks)
- ethergo/examples/signer-example/config/config.go (1 hunks)
- ethergo/examples/signer-example/main.go (1 hunks)
- ethergo/examples/signer-example/metadata/metadata.go (1 hunks)
- ethergo/forker/fork_test.go (2 hunks)
- ethergo/go.mod (4 hunks)
- ethergo/listener/suite_test.go (3 hunks)
- ethergo/signer/config/signer.go (6 hunks)
- ethergo/submitter/submitter_test.go (3 hunks)
- ethergo/submitter/suite_test.go (4 hunks)
Files skipped from review due to trivial changes (7)
- ethergo/examples/contracttests/contracttype.go
- ethergo/examples/contracttests/contracttypeimpl_string.go
- ethergo/examples/contracttests/doc.go
- ethergo/examples/signer-example/README.md
- ethergo/examples/signer-example/config/config.go
- ethergo/examples/signer-example/main.go
- ethergo/submitter/suite_test.go
Additional context used
Hadolint
docker/signer-example.Dockerfile
[warning] 1-1: Using latest is prone to errors if the image will ever update. Pin the version explicitly to a release tag (DL3007)
Markdownlint
ethergo/README.md
13-13: Expected: 0; Actual: 1 (MD007, ul-indent)
Unordered list indentation
14-14: Expected: 0; Actual: 1 (MD007, ul-indent)
Unordered list indentation
15-15: Expected: 0; Actual: 1 (MD007, ul-indent)
Unordered list indentation
16-16: Expected: 0; Actual: 1 (MD007, ul-indent)
Unordered list indentation
17-17: Expected: 0; Actual: 1 (MD007, ul-indent)
Unordered list indentation
6-6: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
21-21: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
59-59: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank linesdocs/bridge/docs/Services/Signer.md
16-16: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
46-46: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
106-106: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
57-57: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
17-17: null (MD024, no-duplicate-heading)
Multiple headings with the same content
108-108: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
115-115: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
58-58: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
11-11: null (MD034, no-bare-urls)
Bare URL used
19-19: null (MD034, no-bare-urls)
Bare URL used
58-58: null (MD034, no-bare-urls)
Bare URL usedethergo/examples/contracttests/README.md
22-22: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
27-27: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
30-30: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
33-33: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
35-35: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
57-57: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
59-59: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
71-71: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
73-73: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
79-79: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
81-81: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
93-93: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
95-95: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
106-106: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
108-108: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
LanguageTool
docs/bridge/docs/Services/Signer.md
[grammar] ~5-~5: The word “setup” is a noun. The verb is spelled with a white space. (NOUN_VERB_CONFUSION)
Context: ...ner ## Setup with AWS KMS In order to setup the signer with AWS KMS, you will need ...
[grammar] ~5-~5: The word “setup” is a noun. The verb is spelled with a white space. (NOUN_VERB_CONFUSION)
Context: ...this, you can follow the steps below to setup the KMS key and the user. Note: If...
[uncategorized] ~7-~7: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: .... Note: If you already have an IAM user you can skip to the [Create a KMS Key](...
[uncategorized] ~7-~7: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...section. If you have a custom IAM setup/role skip this and assign the permissions to...
[grammar] ~33-~33: There may an error in the verb form ‘be create’. (MD_BE_NON_VBP)
Context: ...iam-preview-user.png) 5. The user will be create and appear in the list of users. Click ...
[uncategorized] ~67-~67: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...rify: We will be using the key to sign transactions so we select this option. - Key Spec:
...
[uncategorized] ~78-~78: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE_2)
Context: ...key in the signer. The field is required but the value doesn't matter. - Description...
[uncategorized] ~94-~94: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE)
Context: ...review.png) 7. Your key will be created and you will be redirected to the key detai...
[uncategorized] ~104-~104: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE)
Context: ...key. Note: this is a very basic example and you should not use this in production. ...
[uncategorized] ~107-~107: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...tion and use it securely.signer.yml
: ```yaml signer_config: type: "aws" ...
[uncategorized] ~114-~114: Loose punctuation mark. (UNLIKELY_OPENING_PUNCTUATION)
Context: ...ile: "/path/to/aws.yaml"`aws.yaml`:
yaml region: us-east-1 access_key: Y...ethergo/examples/contracttests/README.md
[style] ~34-~34: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it. (A_VARIETY_OF)
Context: ...ptional) add sanity checks. Go provides a variety of compile/runtime sanity checks through b...
[style] ~34-~34: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing. (REP_WANT_TO_VB)
Context: ... build ./...has been updated. We also want to add an
AllContractTypes` method. These...
[uncategorized] ~80-~80: A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Context: ...t up is Name. We already set this using stringer so we can just call the stringer method...
[misspelling] ~94-~94: Did you mean “too”? (TO_TOO)
Context: ...Info`. This is used when we upload data to tenderly. If you try this make sure tha...
[uncategorized] ~94-~94: A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Context: ... we upload data to tenderly. If you try this make sure thatcounter.Contracts
is c...
[uncategorized] ~94-~94: “do” seems less likely than “to”. (AI_HYDRA_LEO_CP_DO_TO)
Context: ...acts` is correctly imported to the path do your contract. If you need help figurin...
[uncategorized] ~94-~94: You might be missing the article “the” here. (AI_EN_LECTOR_MISSING_DETERMINER_THE)
Context: ... outx.contractinfo.json
wherex
is filename from above ```go // C...
[typographical] ~107-~107: Consider adding a comma here. (FOR_NOW_COMMA)
Context: ... deal with complex initializations, but for now we'll juse use a simple delpoyer in [de...
[style] ~200-~200: Consider a more expressive alternative. (DO_ACHIEVE)
Context: ...ally create a typecast getter. To do this, we're going to create a thin wrap...
Gitleaks
docs/bridge/docs/Services/Signer.md
119-119: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
Additional comments not posted (29)
ethergo/examples/signer-example/metadata/metadata.go (1)
1-15
: Well-structured metadata implementation.The
metadata
package and itsBuildInfo
function are well-implemented, encapsulating versioning and build information effectively.ethergo/examples/contracttests/deploymanager.go (1)
Line range hint
1-24
: Well-implementedDeployManager
for contract tests.The
DeployManager
class is well-structured, providing clear and concise methods for managing deployments.ethergo/deployer/deployed_contract_test.go (1)
Line range hint
1-35
: Comprehensive tests for contract deployment.The test cases in
TestNewDeployedContract
effectively validate the deployment process and the integrity of the deployed contracts.ethergo/examples/contracttests/deployer.go (1)
Line range hint
1-35
: Effective implementation ofCounterDeployer
.The
CounterDeployer
class is well-designed, providing necessary functionalities to deploy and manage the counter contracts efficiently.ethergo/backends/anvil/suite_test.go (2)
4-5
: Updated import paths to reflect new directory structure.
52-53
: Correctly updated function calls to match new package structure.The function calls have been updated to use the new
contracttests
package instead of the oldexample
package. This aligns with the PR's goal of renaming packages for clarity.ethergo/forker/fork_test.go (2)
8-9
: Updated import paths to reflect new directory structure.
21-22
: Correctly updated function calls to match new package structure.The function calls have been updated to use the new
contracttests
package instead of the oldexample
package. This change is consistent across the project, improving clarity and maintainability.ethergo/examples/signer-example/cmd/cmd.go (1)
4-13
: New imports introduced for CLI and configuration handling.The imports are well-organized and relevant to the functionalities being implemented in this file.
ethergo/examples/contracttests/counter_test.go (3)
8-9
: Updated import paths to reflect new directory structure.
22-26
: Correctly updated function calls to match new package structure.The function calls have been updated to use the new
contracttests
package instead of the oldexample
package. This change is consistent across the project, improving clarity and maintainability.
51-51
: Correct deployment manager instantiation in test dependencies.ethergo/.goreleaser.yml (2)
8-30
: New build configuration for Linux AMD64.The configuration is detailed and considers various aspects like static linking and environment variables, ensuring compatibility with the target deployment environment.
31-47
: Docker configuration for Linux AMD64 set up.The Docker configuration includes multiple image templates and build flags, which are well thought out to ensure proper metadata and versioning.
ethergo/listener/suite_test.go (3)
9-10
: Update of import paths due to package renaming.The renaming from
example
tocontracttests
has been correctly reflected in the import paths. This is consistent with the project-wide refactoring.
34-34
: Update of object type due to package renaming.The
DeployManager
type has been updated tocontracttests.DeployManager
to reflect the new package structure. This change is necessary for the code to function correctly with the new package names.
56-56
: Correct instantiation of newDeployManager
.The instantiation of
DeployManager
from thecontracttests
package aligns with the updated package structure. This ensures that the test suite uses the correct types and functions from the newly structured packages.ethergo/README.md (1)
19-19
: Addition of a link to theexamples
directory.The new link to the
examples
directory provides users with a direct way to access example implementations, enhancing the documentation's usability.docs/bridge/docs/Services/Signer.md (1)
3-5
: Clarification in AWS KMS setup documentation.The steps for setting up AWS KMS are clearly outlined, enhancing the documentation's clarity and usefulness for users needing to configure AWS KMS.
Tools
LanguageTool
[grammar] ~5-~5: The word “setup” is a noun. The verb is spelled with a white space. (NOUN_VERB_CONFUSION)
Context: ...ner ## Setup with AWS KMS In order to setup the signer with AWS KMS, you will need ...
[grammar] ~5-~5: The word “setup” is a noun. The verb is spelled with a white space. (NOUN_VERB_CONFUSION)
Context: ...this, you can follow the steps below to setup the KMS key and the user. Note: If...ethergo/signer/config/signer.go (4)
20-20
: Addition of theslices
package.The use of the
slices
package is appropriate for modern Go practices, especially for operations likeContainsFunc
which are used in theIsValid
method.
36-48
: Enhanced validation logic inIsValid
method.The method now correctly checks if the provided signer type is valid by comparing it against a list of all signer types. This robust validation ensures that only supported signer types are used.
71-74
: Addition oflString
method for case-insensitive comparisons.This method facilitates case-insensitive comparisons of signer types, which is particularly useful in configurations where case sensitivity can lead to errors.
Line range hint
103-123
: RefactoredSignerFromConfig
to handle different signer types.The switch statement handles different configurations based on the signer type. This modular approach allows easy extension and maintenance of the code.
ethergo/examples/contracttests/README.md (2)
156-160
: Updated import paths and usage in test examples.The changes reflect the updated package structure and ensure that the test examples use the correct paths and functionalities from the new
examples
andcounter
packages.
236-236
: Introduction of a type-safe getter for contract handles.The addition of
GetCounter
as a type-safe method to retrieve contract handles enhances the robustness and maintainability of the code by reducing the risk of runtime type assertion errors.ethergo/go.mod (2)
57-57
: New dependency added: github.com/urfave/cli/v2This library is commonly used for building CLI applications. Ensure compatibility with existing CLI dependencies and check the library's license for compliance.
71-71
: Dependency version updated: gopkg.in/yaml.v3Upgrading from
yaml.v2
toyaml.v3
can introduce breaking changes due to syntax and functionality differences. Verify that all YAML parsing in the project is compatible with the new version.Verification successful
Dependency version updated: gopkg.in/yaml.v3
The upgrade from
yaml.v2
toyaml.v3
appears to be compatible with the existing codebase. The usage ofyaml.Unmarshal
andyaml.Marshal
in various files follows standard patterns that should work withyaml.v3
.
- Instances of
yaml.Unmarshal
andyaml.Marshal
found in:
services/scribe/config/config.go
services/omnirpc/config/config.go
services/rfq/relayer/relconfig/config.go
services/rfq/api/config/config.go
services/explorer/consumer/parser/utils.go
services/explorer/config/server/config.go
services/explorer/config/indexer/config.go
services/cctp-relayer/config/config.go
ethergo/examples/signer-example/cmd/cmd.go
ethergo/submitter/config/config_test.go
ethergo/signer/config/signer.go
ethergo/signer/signer/awssigner/kmsmock/internal/seed_gen.go
contrib/promexporter/config/config.go
contrib/opbot/cmd/commands.go
contrib/screener-api/cmd/commands.go
agents/config/executor/config.go
agents/config/agent_config.go
No issues were found with the compatibility of these instances with
yaml.v3
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for YAML parsing code to verify compatibility with yaml.v3. # Test: Search for YAML parsing code. Expect: Only compatible code with yaml.v3 should be found. rg --type go --glob '*.go' 'yaml.Unmarshal|yaml.Marshal'Length of output: 1988
Script:
#!/bin/bash # Description: Check the context of YAML parsing code to verify compatibility with yaml.v3. # Extract lines around yaml.Unmarshal and yaml.Marshal to verify compatibility. rg --type go --glob '*.go' -A 5 -B 5 'yaml.Unmarshal|yaml.Marshal'Length of output: 19444
ethergo/submitter/submitter_test.go (2)
5-6
: Updated import paths to reflect the new package structure.The import paths have been updated to reflect the renaming of packages from
example
tocontracttests
. This change aligns with the project's efforts to improve naming conventions and structure.
273-273
: Refactored test to use new package structure.The test function has been updated to use the new
contracttests.CounterType
instead of the oldexample.CounterType
. This change is necessary to maintain consistency with the updated project structure.
@@ -0,0 +1,15 @@ | |||
FROM gcr.io/distroless/static:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider pinning the Docker base image version.
Using latest
can lead to unexpected behaviors if the base image is updated. It's safer to pin the image to a specific version.
- FROM gcr.io/distroless/static:latest
+ FROM gcr.io/distroless/static:<specific-version>
Committable suggestion was skipped due to low confidence.
Tools
Hadolint
[warning] 1-1: Using latest is prone to errors if the image will ever update. Pin the version explicitly to a release tag (DL3007)
var action = func(c *cli.Context) error { | ||
configFile, err := os.ReadFile(c.String(fileFlag.Name)) | ||
if err != nil { | ||
return fmt.Errorf("failed to open config file: %w", err) | ||
} | ||
|
||
var cfg exampleConfig.ExampleConfig | ||
// nolint: musttag | ||
err = yaml.Unmarshal(configFile, &cfg) | ||
if err != nil { | ||
return fmt.Errorf("failed to unmarshal config file: %w", err) | ||
} | ||
|
||
if ok, err := cfg.SignerConfig.IsValid(c.Context); !ok { | ||
panic(fmt.Errorf("failed to validate signer config: %w", err)) | ||
} | ||
|
||
signer, err := signerConfig.SignerFromConfig(c.Context, cfg.SignerConfig) | ||
if err != nil { | ||
return fmt.Errorf("could not create signer: %w", err) | ||
} | ||
|
||
fmt.Printf("signer address is %s", signer.Address()) | ||
|
||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Command action setup with configuration loading and validation.
This segment properly handles configuration loading and validation. However, the use of panic
in line 56 could be replaced with a more user-friendly error handling approach.
- panic(fmt.Errorf("failed to validate signer config: %w", err))
+ fmt.Println("Error validating signer config:", err)
+ return err
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.
var action = func(c *cli.Context) error { | |
configFile, err := os.ReadFile(c.String(fileFlag.Name)) | |
if err != nil { | |
return fmt.Errorf("failed to open config file: %w", err) | |
} | |
var cfg exampleConfig.ExampleConfig | |
// nolint: musttag | |
err = yaml.Unmarshal(configFile, &cfg) | |
if err != nil { | |
return fmt.Errorf("failed to unmarshal config file: %w", err) | |
} | |
if ok, err := cfg.SignerConfig.IsValid(c.Context); !ok { | |
panic(fmt.Errorf("failed to validate signer config: %w", err)) | |
} | |
signer, err := signerConfig.SignerFromConfig(c.Context, cfg.SignerConfig) | |
if err != nil { | |
return fmt.Errorf("could not create signer: %w", err) | |
} | |
fmt.Printf("signer address is %s", signer.Address()) | |
return nil | |
} | |
var action = func(c *cli.Context) error { | |
configFile, err := os.ReadFile(c.String(fileFlag.Name)) | |
if err != nil { | |
return fmt.Errorf("failed to open config file: %w", err) | |
} | |
var cfg exampleConfig.ExampleConfig | |
// nolint: musttag | |
err = yaml.Unmarshal(configFile, &cfg) | |
if err != nil { | |
return fmt.Errorf("failed to unmarshal config file: %w", err) | |
} | |
if ok, err := cfg.SignerConfig.IsValid(c.Context); !ok { | |
fmt.Println("Error validating signer config:", err) | |
return err | |
} | |
signer, err := signerConfig.SignerFromConfig(c.Context, cfg.SignerConfig) | |
if err != nil { | |
return fmt.Errorf("could not create signer: %w", err) | |
} | |
fmt.Printf("signer address is %s", signer.Address()) | |
return nil | |
} |
func Start(args []string, buildInfo config.BuildInfo) { | ||
app := cli.NewApp() | ||
app.Name = buildInfo.Name() | ||
|
||
app.Description = buildInfo.VersionString() + "Opbot is a tool to manage operations." | ||
app.Usage = fmt.Sprintf("%s --help", buildInfo.Name()) | ||
app.EnableBashCompletion = true | ||
app.Before = func(c *cli.Context) error { | ||
// nolint:wrapcheck | ||
return metrics.Setup(c.Context, buildInfo) | ||
} | ||
app.Flags = []cli.Flag{fileFlag} | ||
app.Action = action | ||
err := app.Run(args) | ||
|
||
if err != nil { | ||
panic(err) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CLI setup implemented with robust error handling and configuration.
The CLI is set up with comprehensive options and error handling. Using panic
for error handling in line 32 might be reconsidered for production code to allow more graceful error recovery.
- panic(err)
+ fmt.Println("Error:", err)
+ os.Exit(1)
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.
func Start(args []string, buildInfo config.BuildInfo) { | |
app := cli.NewApp() | |
app.Name = buildInfo.Name() | |
app.Description = buildInfo.VersionString() + "Opbot is a tool to manage operations." | |
app.Usage = fmt.Sprintf("%s --help", buildInfo.Name()) | |
app.EnableBashCompletion = true | |
app.Before = func(c *cli.Context) error { | |
// nolint:wrapcheck | |
return metrics.Setup(c.Context, buildInfo) | |
} | |
app.Flags = []cli.Flag{fileFlag} | |
app.Action = action | |
err := app.Run(args) | |
if err != nil { | |
panic(err) | |
} | |
} | |
func Start(args []string, buildInfo config.BuildInfo) { | |
app := cli.NewApp() | |
app.Name = buildInfo.Name() | |
app.Description = buildInfo.VersionString() + "Opbot is a tool to manage operations." | |
app.Usage = fmt.Sprintf("%s --help", buildInfo.Name()) | |
app.EnableBashCompletion = true | |
app.Before = func(c *cli.Context) error { | |
// nolint:wrapcheck | |
return metrics.Setup(c.Context, buildInfo) | |
} | |
app.Flags = []cli.Flag{fileFlag} | |
app.Action = action | |
err := app.Run(args) | |
if err != nil { | |
fmt.Println("Error:", err) | |
os.Exit(1) | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
- Added detailed documentation for AWS KMS signer setup
- Created Dockerfile for signer-example application
- Updated GoReleaser configuration for signer example
- Renamed and reorganized example directory to examples
- Updated import paths and references in test files to match new structure
14 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings
docs/bridge/docs/Services/Signer.md
Outdated
|
||
- Alias: This is an alias that will be used to reference the key in the signer. The field is required but the value doesn't matter. | ||
- Description: This is a description that will be used to describe the key in the signer. This is only optional and only for your reference. | ||
- Tags: These are tags that will be used to describe the key in the signer. This ese are optional and only for your reference. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📚 spelling: 'ese are optional' should be 'these are optional'.
- Tags: These are tags that will be used to describe the key in the signer. This ese are optional and only for your reference. | |
These are optional and only for your reference. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Updated Dockerfile path in
.goreleaser.yml
- Added
nolint
directive incmd.go
- Documented AWS signer in
signer.go
- Modified
lString
method for case-insensitive comparisons
3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
This reverts commit 3f20c96.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Added documentation for AWS signer
- Included setup instructions for AWS KMS configuration
- Introduced a simple signer configuration example
- Updated example folder links in
README.md
- Updated import paths and package references in test and configuration files
No file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
- Added detailed setup instructions for configuring the signer with AWS KMS in
Signer.md
- Included example configurations and commands for running the signer locally or via Docker
- Updated example folder links in
README.md
to reflect the correct directory structure - Updated import paths and package references from
example
tocontracttests
in various test and configuration files - Added new dependencies:
github.com/urfave/cli/v2
andgopkg.in/yaml.v3
1 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings
|
||
**Note**: If you already have an IAM user you can skip to the [Create a KMS Key](#create-a-kms-key) section. If you have a custom IAM setup/role skip this and assign the permissions to the user or role you want to use. | ||
|
||
### Create an IAM User |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🪶 style: Duplicate section title 'Create an IAM User'. Consider removing one.
|
||
- Alias: This is an alias that will be used to reference the key in the signer. The field is required but the value doesn't matter. | ||
- Description: This is a description that will be used to describe the key in the signer. This is only optional and only for your reference. | ||
- Tags: These are tags that will be used to describe the key in the signer. These are optional and only for your reference. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📚 spelling: 'ese are optional' should be 'these are optional'.
- Tags: These are tags that will be used to describe the key in the signer. These are optional and only for your reference. | |
These are optional and only for your reference. |
- Description: This is a description that will be used to describe the key in the signer. This is only optional and only for your reference. | ||
- Tags: These are tags that will be used to describe the key in the signer. These are optional and only for your reference. | ||
|
||
4. Choose `Next` and define your key admin permissions. These are *not* required by the signer itself but are required to manage the key. You can use whatever you like here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📚 spelling: 'sigenr' is misspelled. Correct to 'signer'.
4. Choose `Next` and define your key admin permissions. These are *not* required by the signer itself but are required to manage the key. You can use whatever you like here. | |
signer |
Description
Add documentation for AWS signer
d1e19fb: docs preview link
Summary by CodeRabbit
New Features
Documentation
README.md
to reflect the correct directory structure.Chores
example
tocontracttests
in various test and configuration files.github.com/urfave/cli/v2
andgopkg.in/yaml.v3
.Refactor
ee925ba: docs preview link
09e1a88: docs preview link
fee4979: docs preview link