Revise the print-config command #13679
Conversation
…tor into jmacd/print_command
…tor into jmacd/print_command
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (83.07%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #13679 +/- ##
==========================================
- Coverage 91.52% 91.46% -0.06%
==========================================
Files 639 639
Lines 41662 41767 +105
==========================================
+ Hits 38131 38204 +73
- Misses 2709 2732 +23
- Partials 822 831 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…tor into jmacd/print_command
…tor into jmacd/print_command
…collector into jmacd/print_command
| // printUnredactedConfig prints resolved configuration before interpreting | ||
| // with the intended types for each component, thus it shows the full | ||
| // configuration without considering configuopaque. Use with caution. | ||
| func (pctx *printContext) printUnredactedConfig() error { |
There was a problem hiding this comment.
Can this be a CVE? I am thinking of a scenario where via the otelcol someone gets access to some credentials (VoltDB, etc.)?
I have not spent too much time thinking of all possible scenarios but I am worried a bit to offer this.
There was a problem hiding this comment.
It's a fair question. I came to this PR from a tough debugging session, #13273, where I was hit by a bug in the underlying mapstructure library and couldn't see the configuration that was invalid (fixed upstream, unreleased). This led me to #13278.
I looked at the existing print-initial-config command, and found:
- it had been written by a similarly frustrated user wanting to see "initial" meaning unredacted configuration
- however, it was broken because it didn't fully resolve, so you couldn't see default settings.
- there is a feature flag covering the existing print-initial-config command (broken it may be), because of your concern about the CVE
This was discussed in a Collector SIG once or twice. The synopsis was that if a user has the power to run the collector and resolve the configuration, we're not really securing anything by hiding this detail, but we are sometimes making the operator's life difficult.
My idea was to embrace the operator need and build in protections so that by default we apply the configopaque mechanism, which is meant as a solution to exactly this problem. So, I think no, this is not a CVE, this is secure-by-default and good for users. 👍
…tor into jmacd/print_command
…collector into jmacd/print_command
| # (Optional) One or more lines of additional information to render under the primary note. | ||
| # These lines will be padded with 2 spaces and then inserted directly into the document. | ||
| # Use pipe (|) for multiline entries. | ||
| subtext: |
There was a problem hiding this comment.
| subtext: | |
| subtext: This replaces the `print-initial-config` command. See the `service` package README for more details. |
I think we should make sure users who might be using the previous command are aware it's replaced. Also would be nice to point users to where to find more.
| # Use this changelog template to create an entry for release notes. | ||
|
|
||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
| change_type: enhancement |
There was a problem hiding this comment.
| change_type: enhancement | |
| change_type: breaking |
I doubt it will break many people, but this is still realistically a breaking change (even behind a feature gate) since we're renaming the command.
|
|
||
| Validation is enabled by default, as a safety measure. | ||
|
|
||
| All modes are experimental requiring otelcol.printInitialConfig feature gate.`, |
There was a problem hiding this comment.
| All modes are experimental requiring otelcol.printInitialConfig feature gate.`, | |
| All modes are experimental, requiring the otelcol.printInitialConfig feature gate.`, |
| valid. To print a potentially invalid configuration, use | ||
| `--validate=false`. | ||
|
|
There was a problem hiding this comment.
| Note that by default the configuration will only print when it is | |
| valid. To print a potentially invalid configuration, use | |
| `--validate=false`. |
Could we either remove this instance of this paragraph, or look at moving it elsewhere? I understand putting it in both sections, but having it repeated not long after the previous one makes it feel like an editing mishap.
We could also try to put it in a "caution" box if you think it's important enough to repeat.
There was a problem hiding this comment.
Looks like this got committed by accident, we should delete it.
Description
See #12369, #11775.
Fixes #13278.
cc/ @VihasMakwana
This refines the print-config command with two modes, "redacted" and "unredacted", with an optional "--validate" or no, with optional --format=yaml or json.
Link to tracking issue
Fixes #11775.
Testing
✔️
Documentation
✔️