-
Notifications
You must be signed in to change notification settings - Fork 68
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
Helm chart chain id fix #299
Conversation
… encoded so putting decimal or hexadecimal values in there will often render something like this: % Moving this to single quoted value fixes that. Signed-off-by: Matt Halder <[email protected]>
…decimal value not hexadecimal. Signed-off-by: Matt Halder <[email protected]>
78bf43e
to
e616bef
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Codecov Report
@@ Coverage Diff @@
## main #299 +/- ##
=======================================
Coverage 57.81% 57.81%
=======================================
Files 9 9
Lines 825 825
Branches 131 131
=======================================
Hits 477 477
Misses 322 322
Partials 26 26 Continue to review full report at Codecov.
|
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.
LG
@@ -10,7 +10,6 @@ data: | |||
OPERATOR_KEY_MAIN: {{ .Values.config.OPERATOR_KEY_MAIN | b64enc }} | |||
OPERATOR_ID_ETH_SENDRAWTRANSACTION: {{ .Values.config.OPERATOR_ID_ETH_SENDRAWTRANSACTION | default (printf "%q" "") }} | |||
OPERATOR_KEY_ETH_SENDRAWTRANSACTION: {{ .Values.config.OPERATOR_KEY_ETH_SENDRAWTRANSACTION | default (printf "%q" "") }} |
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.
Why are these operator properties not base64 encoded and are just escaped? Better to use stringData
instead of data
to avoid all the extra base64 conversion anyway.
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.
Agreed. @rustyShacklefurd feel free to open a separate PR that addresses this properly.
This satisfies the work around
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.
Added #301 to capture this
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.
good suggestion. I will put this change in new branch and get it PR'd
Description:
Cleanup the way CHAIN_ID is set and transported into the k8s deployments. Hex numbers are often converted (or attempted to be converted) and this makes the code return 0xNaN as the CHAIN_ID. Moving away from setting this in a k8s secret because there is less conversion/encoding.
Fixes #300