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.

Sign up for GitHub

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

revised how custom config is used #20

Merged
merged 1 commit into from
Dec 25, 2023
Merged

revised how custom config is used #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions cmd/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func main() {
var customConfigPath string
flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
flag.StringVar(&customConfigPath, "custom-config-path", "./custom/config.yaml", "the path to custom config.")
flag.StringVar(&customConfigPath, "custom-config-path", "", "the path to custom config.")
flag.BoolVar(&enableLeaderElection, "leader-elect", false,
"Enable leader election for controller manager. "+
"Enabling this will ensure there is only one active controller manager.")
Expand All @@ -69,12 +69,19 @@ func main() {
flag.Parse()

ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
customConfig, err := config.InitConfig(customConfigPath)
if err != nil {
setupLog.Info("custom config not loaded")
} else {
authenticatorv1alpha1.ValidationTimeout = time.Second * time.Duration(customConfig.WebhookConf.ValidationTimeoutSecond)

var customConfig *config.CustomConfig
if customConfigPath != "" {
setupLog.Info(customConfigPath)
tmpConf, err := config.InitConfig(customConfigPath)
if err != nil {
setupLog.Error(err, "failed to load custom config")
} else {
customConfig = tmpConf
authenticatorv1alpha1.ValidationTimeout = time.Second * time.Duration(customConfig.WebhookConf.ValidationTimeoutSecond)
}
}

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
Scheme: scheme,
MetricsBindAddress: metricsAddr,
Expand Down
1 change: 1 addition & 0 deletions config/default/manager_auth_proxy_patch.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,4 @@ spec:
- "--health-probe-bind-address=:8081"
- "--metrics-bind-address=127.0.0.1:8080"
- "--leader-elect"
- $(ARGS)
6 changes: 4 additions & 2 deletions internal/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package config

import "github.com/spf13/viper"
import (
"github.com/spf13/viper"
)

type CustomConfig struct {
WebserverConf WebserverConfig `mapstructure:"webserver"`
Expand All @@ -17,7 +19,7 @@ type WebhookConfig struct {
}

func InitConfig(configPath string) (*CustomConfig, error) {
viper.AddConfigPath(configPath)
viper.SetConfigFile(configPath)
viper.SetConfigType("yaml")
err := viper.ReadInConfig()
if err != nil {
Comment on lines 21 to 25
Copy link

@coderabbitai coderabbitai bot Dec 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The change from viper.AddConfigPath to viper.SetConfigFile requires that configPath is a full path to the configuration file. Ensure that all invocations of InitConfig are updated to pass the full file path, not just a directory.

Verification successful

The script output confirms that customConfigPath is used to store the path to the custom configuration file, and it is passed directly to config.InitConfig. This aligns with the expectation that configPath should be a full file path. There is no indication that customConfigPath is treated as a directory path, which suggests that the change in InitConfig to use viper.SetConfigFile is consistent with how customConfigPath is being set and used in cmd/main.go.

Scripts Executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for invocations of InitConfig to ensure they pass a full file path.
rg --type go 'InitConfig\('

Length of output: 181

Script:

#!/bin/bash
# Extract the relevant sections from cmd/main.go where customConfigPath is being set or used.
rg --type go 'customConfigPath' cmd/main.go

Length of output: 280

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

its done

Copy link

Choose a reason for hiding this comment

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

@sinamna, thank you for updating the invocations of InitConfig to ensure they pass the full file path. If there's anything else that needs attention, feel free to mention it.

If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

Expand Down