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

feat: improve configuration parsing #24

Merged
merged 18 commits into from
Nov 21, 2023
Merged

Conversation

H777K
Copy link
Member

@H777K H777K commented Nov 20, 2023

This PR ensures, that the configuration of the garm-operator can be done with ENVs, Flags and Config File (yaml)

@H777K H777K self-assigned this Nov 20, 2023

var Config AppConfig

func ReadConfig(f *pflag.FlagSet, configFile string) error {
Copy link
Member

Choose a reason for hiding this comment

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

ReadConfig reads more like reading the configuration from file, but we are doing more a generating of the configuration in this case,

Therefore i would suggest renaming to:

Suggested change
func ReadConfig(f *pflag.FlagSet, configFile string) error {
func GenerateConfig(f *pflag.FlagSet, configFile string) error {

Copy link
Member

@bavarianbidi bavarianbidi left a comment

Choose a reason for hiding this comment

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

just one nit - #24 (comment)

but i will approve it anyway

README.md Outdated
curl -L https://github.com/mercedes-benz/garm-operator/releases/download/${GARM_OPERATOR_VERSION}/garm-operator-all.yaml | envsubst | kubectl apply -f -
```

The Full Configuration Parsing Documentation can be found in the [configuration parsing guide](./docs/config/configuration-parsing.md)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The Full Configuration Parsing Documentation can be found in the [configuration parsing guide](./docs/config/configuration-parsing.md)
The full configuration parsing documentation can be found in the [configuration parsing guide](./docs/config/configuration-parsing.md)


# Configuration Parsing

The Configuration Parsing for the Garm Operator is implemented with [koanf](https://github.com/knadh/koanf).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The Configuration Parsing for the Garm Operator is implemented with [koanf](https://github.com/knadh/koanf).
The configuration parsing for the Garm Operator is implemented with [koanf](https://github.com/knadh/koanf).


The Configuration Parsing for the Garm Operator is implemented with [koanf](https://github.com/knadh/koanf).

The Configuration can be defined with `ENVs`, `Flags` and `config file (yaml)`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The Configuration can be defined with `ENVs`, `Flags` and `config file (yaml)`.
The configuration can be defined with `ENVs`, `Flags` and `config file (yaml)`.


For the Garm Operator the following order is defined, which is to be considered in ascending order from lowest to highest priority:

1. Defined Default Values ([see section Configuration Default Values](#configuration-default-values))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
1. Defined Default Values ([see section Configuration Default Values](#configuration-default-values))
1. Defined default values ([see section configuration default values](#configuration-default-values))

--dry-run
```

The `--config` flag can be set to specify the path to the `config file (yaml)` which contains the configuration ([see section Config File Strucutre](#config-file-structure)).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The `--config` flag can be set to specify the path to the `config file (yaml)` which contains the configuration ([see section Config File Strucutre](#config-file-structure)).
The `--config` flag can be set to specify the path to the `config file (yaml)` which contains the configuration ([see section config file structure](#config-file-structure)).


The `--config` flag can be set to specify the path to the `config file (yaml)` which contains the configuration ([see section Config File Strucutre](#config-file-structure)).

The `--dry-run` flag can be set to show the parsed configuration, without starting the Garm Operator. The Output can be similar to the following:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The `--dry-run` flag can be set to show the parsed configuration, without starting the Garm Operator. The Output can be similar to the following:
The `--dry-run` flag can be set to show the parsed configuration, without starting the Garm Operator. The output can be similar to the following:


## Parsing Validation

After the Configuration has been parsed by koanf and unmarshelled into a struct, the [validator](https://github.com/go-playground/validator) checks whether the generated struct is valid or not.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
After the Configuration has been parsed by koanf and unmarshelled into a struct, the [validator](https://github.com/go-playground/validator) checks whether the generated struct is valid or not.
After the configuration has been parsed by koanf and unmarshalled into a struct, the [validator](https://github.com/go-playground/validator) checks whether the generated struct is valid or not.

@H777K H777K marked this pull request as ready for review November 21, 2023 14:52
@H777K H777K merged commit 9a99d0d into main Nov 21, 2023
1 check passed
@H777K H777K deleted the feat/improve-configuration-parsing branch November 21, 2023 14:58
@H777K H777K mentioned this pull request Nov 29, 2023
@H777K H777K mentioned this pull request Nov 29, 2023
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.

2 participants