feat: Add manifest update Command for Configuration Management#308
feat: Add manifest update Command for Configuration Management#308
Conversation
|
|
||
| var manifestConfigPath string | ||
|
|
||
| // manifestCmd represents the manifest command |
There was a problem hiding this comment.
Reminder to align on our team's 'philosophy' on comments
| Long: ` | ||
| The manifest command allows you to manage the OCB manifest file.`, | ||
| Run: func(cmd *cobra.Command, args []string) { | ||
| fmt.Println("manifest called") |
There was a problem hiding this comment.
Intentional or is this a leftover debugging artifact?
There was a problem hiding this comment.
I think it's leftover from the cobra scaffolding
| manifestCmd.AddCommand(manifest.UpdateCmd) | ||
|
|
||
| // Define a persistent flag for `manifestCmd` | ||
| manifestCmd.PersistentFlags().StringVarP( |
There was a problem hiding this comment.
When testing, I absent-mindedly provided the config.yaml instead of the manifest.yaml and it obviously failed. As we have two configs in the collector world and the runtime configs are the ones that usually get more attention and are thus more associated with the 'config' word, we might want to think about using a different name/flag here? Just a thought but wanted to mention it.
| Long: "Update the manifest file to ensure otel components are up to date.", | ||
|
|
||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| fmt.Println("manifest update called") |
| var errMissingGoMod = errors.New("missing gomod specification for module") | ||
|
|
||
| // Config holds the builder's configuration | ||
| type Config struct { |
There was a problem hiding this comment.
I assume there wasn't a (easy) way to reuse the definitions from the otel repo?
There was a problem hiding this comment.
Nope, all structs are internal
| func TestUpdateCmd_RunE_InvalidConfig(t *testing.T) { | ||
|
|
||
| cmd := &cobra.Command{} | ||
| cmd.Flags().String("config", "test-config.yaml", "") |
There was a problem hiding this comment.
Is there a reason we don't use the invalid test data config? As far as I can tell it's unused?
| assert.Contains(t, err.Error(), "invalid configuration") | ||
|
|
||
| mockConfig.AssertExpectations(t) | ||
| } |
There was a problem hiding this comment.
If we support globbing, a test to validate that would be great.
| func validateModules(name string, mods []Module) error { | ||
| for i, mod := range mods { | ||
| if mod.GoMod == "" { | ||
| return fmt.Errorf("%s module at index %v: %w", name, i, errMissingGoMod) |
There was a problem hiding this comment.
When I tried to accidentally parse a non-manifest yaml, the error message was
2025-05-12T14:35:58.717-0700 INFO manifest/update.go:80 Using config file {"path": "../../distributions/nrdot-collector-host/config.yaml"}
Error: invalid configuration: extension module at index 0: missing gomod specification for module; receiver module at index 0: missing gomod specification for module; exporter module at index 0: missing gomod specification for module; processor module at index 0: missing gomod specification for module
It took me a while to realize that I was parsing the wrong type of config. Might be good to short circuit early when the component-type keys (e.g. receiver) is not present at all instead of accumulating all the errors from this level.
| return nil | ||
| } | ||
|
|
||
| func parseModules(mods []Module, usedNames map[string]int) ([]Module, error) { |
There was a problem hiding this comment.
As we ourselves are not yet using anything but gomod for module specification, it would be good to have some tests to document what this is validating.
mailo-nr
force-pushed
the
marsac/nrdot-cli
branch
from
b00de65 to
1e142a6
Compare
2 participants
This PR introduces a new manifest update command to the nrdot-collector-builder CLI. The command streamlines the process of updating manifest configuration files, ensuring that OpenTelemetry (otel) components are always up to date and that configuration files remain valid and consistent.
New Cobra Command:
Adds a manifest update subcommand, accessible via the CLI, with clear usage and help text.
Flexible File Matching:
Supports glob patterns for the --config flag, allowing users to update multiple manifest files in a single invocation.
Robust Validation:
Loads the specified configuration file(s), validates their structure and content, and ensures all required fields and modules are present.
Go Environment Detection:
Automatically detects and sets the correct Go binary path if not explicitly provided, improving portability and reliability.
Example Usage: