- Notifications
You must be signed in to change notification settings - Fork 70
Enable passing config into controller #264
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
Changes from all commits
a4610f1 80c9ac2 a703b09 2702d21 952991f cf9ea34 bdda6e6 f81282b d443ae0 f37bc66 bab00cb File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: env-config | ||
| data: | ||
| awsRegion: {{ .Values.awsRegion }} | ||
| awsAccountId: {{ .Values.awsAccountId }} | ||
| clusterVpcId: {{ .Values.clusterVpcId }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| | @@ -12,20 +12,28 @@ import ( | |
| const ( | ||
| LatticeGatewayControllerName = "application-networking.k8s.aws/gateway-api-controller" | ||
| defaultLogLevel = "Info" | ||
| NoDefaultServiceNetwork = "" | ||
| NO_DEFAULT_SERVICE_NETWORK = "NO_DEFAULT_SERVICE_NETWORK" | ||
| UnknownInput = "" | ||
| ) | ||
| | ||
| // TODO endpoint, region | ||
| var VpcID = "vpc-xxxx" | ||
| var AccountID = "yyyyyy" | ||
| var Region = "us-west-2" | ||
| const ( | ||
| NO_DEFAULT_SERVICE_NETWORK = "NO_DEFAULT_SERVICE_NETWORK" | ||
| REGION = "REGION" | ||
| CLUSTER_VPC_ID = "CLUSTER_VPC_ID" | ||
| CLUSTER_LOCAL_GATEWAY = "CLUSTER_LOCAL_GATEWAY" | ||
| AWS_ACCOUNT_ID = "AWS_ACCOUNT_ID" | ||
| TARGET_GROUP_NAME_LEN_MODE = "TARGET_GROUP_NAME_LEN_MODE" | ||
| GATEWAY_API_CONTROLLER_LOGLEVEL = "GATEWAY_API_CONTROLLER_LOGLEVEL" | ||
| ) | ||
| | ||
| var VpcID = UnknownInput | ||
| var AccountID = UnknownInput | ||
| var Region = UnknownInput | ||
| var logLevel = defaultLogLevel | ||
| var DefaultServiceNetwork = NoDefaultServiceNetwork | ||
| var DefaultServiceNetwork = UnknownInput | ||
| var UseLongTGName = false | ||
| | ||
| func GetLogLevel() string { | ||
| logLevel = os.Getenv("GATEWAY_API_CONTROLLER_LOGLEVEL") | ||
| logLevel = os.Getenv(GATEWAY_API_CONTROLLER_LOGLEVEL) | ||
| switch strings.ToLower(logLevel) { | ||
| case "debug": | ||
| return "10" | ||
| | @@ -36,86 +44,74 @@ func GetLogLevel() string { | |
| } | ||
| | ||
| func GetClusterLocalGateway() (string, error) { | ||
| if DefaultServiceNetwork == NoDefaultServiceNetwork { | ||
| return NoDefaultServiceNetwork, errors.New(NO_DEFAULT_SERVICE_NETWORK) | ||
| if DefaultServiceNetwork == UnknownInput { | ||
| return UnknownInput, errors.New(NO_DEFAULT_SERVICE_NETWORK) | ||
| } | ||
| | ||
| return DefaultServiceNetwork, nil | ||
| } | ||
| | ||
| func ConfigInit() { | ||
| // discover VPC using environment first | ||
| VpcID = os.Getenv("CLUSTER_VPC_ID") | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any reason to change this? Today we documented how to use these environments variables and they are used for developers as well (e..g make presubmit) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Document says: When running AWS Gateway API Controller This code changes is to take the input while running AWS Gateway API Controller | ||
| glog.V(2).Infoln("CLUSTER_VPC_ID: ", os.Getenv("CLUSTER_VPC_ID")) | ||
| | ||
| // discover Account | ||
| AccountID = os.Getenv("AWS_ACCOUNT_ID") | ||
| if AccountID == "" { | ||
| AccountID = os.Getenv("AWS_ACCOUNT") // Fallback to AWS_ACCOUNT for compatibility | ||
| } | ||
| glog.V(2).Infoln("AWS_ACCOUNT_ID:", AccountID) | ||
| | ||
| // discover Region | ||
| Region = os.Getenv("REGION") | ||
| glog.V(2).Infoln("REGION:", os.Getenv("REGION")) | ||
| | ||
| logLevel = os.Getenv("GATEWAY_API_CONTROLLER_LOGLEVEL") | ||
| glog.V(2).Infoln("Logging Level:", os.Getenv("GATEWAY_API_CONTROLLER_LOGLEVEL")) | ||
| | ||
| DefaultServiceNetwork = os.Getenv("CLUSTER_LOCAL_GATEWAY") | ||
| | ||
| if DefaultServiceNetwork == NoDefaultServiceNetwork { | ||
| glog.V(2).Infoln("No CLUSTER_LOCAL_GATEWAY") | ||
| } else { | ||
| | ||
| glog.V(2).Infoln("CLUSTER_LOCAL_GATEWAY", DefaultServiceNetwork) | ||
| } | ||
| | ||
| tgNameLengthMode := os.Getenv("TARGET_GROUP_NAME_LEN_MODE") | ||
| | ||
| glog.V(2).Infoln("TARGET_GROUP_NAME_LEN_MODE", tgNameLengthMode) | ||
| | ||
| if tgNameLengthMode == "long" { | ||
| UseLongTGName = true | ||
| } else { | ||
| UseLongTGName = false | ||
| } | ||
| | ||
| sess, _ := session.NewSession() | ||
| metadata := NewEC2Metadata(sess) | ||
| | ||
| var err error | ||
| if ifRunningInCluster() { | ||
| | ||
| // CLUSTER_VPC_ID | ||
| VpcID = os.Getenv(CLUSTER_VPC_ID) | ||
| if VpcID != UnknownInput { | ||
| glog.V(2).Infoln("CLUSTER_VPC_ID passed as input:", VpcID) | ||
| } else { | ||
| VpcID, err = metadata.VpcID() | ||
| glog.V(2).Infoln("CLUSTER_VPC_ID from IMDS config discovery :", VpcID) | ||
| if err != nil { | ||
| return | ||
| glog.V(2).Infoln("IMDS config discovery for CLUSTER_VPC_ID is NOT AVAILABLE :", err) | ||
| } | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From what I have seen, the most common practice on config is to have priority order Actually... we have a lot of configuration variables (longname mode for example) and I think they all should be configurable by both flag and env. It makes things bigger so we can leave it to other task though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we have two ways of configuring the same value I think the naming should be consistent e.g. flag There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I discussed with Liwen, we plan to go with env, no flag for now. I am going to change this using env var. | ||
| } | ||
| | ||
| // REGION | ||
| Region = os.Getenv(REGION) | ||
| if Region != UnknownInput { | ||
| glog.V(2).Infoln("REGION passed as input:", Region) | ||
| } else { | ||
| Region, err = metadata.Region() | ||
| glog.V(2).Infoln("REGION from IMDS config discovery :", Region) | ||
| if err != nil { | ||
| return | ||
| glog.V(2).Infoln("IMDS config discovery for REGION is NOT AVAILABLE :", err) | ||
| } | ||
| } | ||
| | ||
| // AWS_ACCOUNT_ID | ||
| AccountID = os.Getenv(AWS_ACCOUNT_ID) | ||
| if AccountID != UnknownInput { | ||
| glog.V(2).Infoln("AWS_ACCOUNT_ID passed as input:", AccountID) | ||
| } else { | ||
| AccountID, err = metadata.AccountId() | ||
| glog.V(2).Infoln("AWS_ACCOUNT_ID from IMDS config discovery :", AccountID) | ||
| if err != nil { | ||
| return | ||
| glog.V(2).Infoln("IMDS config discovery for AWS_ACCOUNT_ID is NOT AVAILABLE :", err) | ||
| } | ||
| glog.V(2).Infoln("INSIDE CLUSTER CLUSTER_VPC_ID: ", VpcID) | ||
| glog.V(2).Infoln("INSIDE CLUSTER REGION: ", Region) | ||
| glog.V(2).Infoln("INSIDE CLUSTER ACCOUNT_ID: ", AccountID) | ||
| } | ||
| } | ||
| | ||
| func ifRunningInCluster() bool { | ||
| _, err := os.Stat("/var/run/secrets/kubernetes.io/serviceaccount") | ||
| if err == nil { | ||
| glog.V(2).Infoln("Controller is running inside cluster") | ||
| return true | ||
| } | ||
| // GATEWAY_API_CONTROLLER_LOGLEVEL | ||
| logLevel = os.Getenv(GATEWAY_API_CONTROLLER_LOGLEVEL) | ||
| glog.V(2).Infoln("Logging Level:", os.Getenv(GATEWAY_API_CONTROLLER_LOGLEVEL)) | ||
| | ||
| if os.IsNotExist(err) { | ||
| glog.V(2).Infoln("Controller is NOT running inside cluster") | ||
| return false | ||
| // CLUSTER_LOCAL_GATEWAY | ||
| DefaultServiceNetwork = os.Getenv(CLUSTER_LOCAL_GATEWAY) | ||
| if DefaultServiceNetwork == UnknownInput { | ||
| glog.V(2).Infoln("No CLUSTER_LOCAL_GATEWAY") | ||
| } else { | ||
| glog.V(2).Infoln("CLUSTER_LOCAL_GATEWAY", DefaultServiceNetwork) | ||
| } | ||
| | ||
| glog.V(2).Infoln("Controller is NOT running inside cluster") | ||
| return false | ||
| // TARGET_GROUP_NAME_LEN_MODE | ||
| tgNameLengthMode := os.Getenv(TARGET_GROUP_NAME_LEN_MODE) | ||
| glog.V(2).Infoln("TARGET_GROUP_NAME_LEN_MODE", tgNameLengthMode) | ||
| | ||
| if tgNameLengthMode == "long" { | ||
| UseLongTGName = true | ||
| } else { | ||
| UseLongTGName = false | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| package config | ||
| | ||
| import ( | ||
| "github.com/stretchr/testify/assert" | ||
| "os" | ||
| "testing" | ||
| ) | ||
| | ||
| func Test_config_init_with_partial_env_var(t *testing.T) { | ||
| // Test variable | ||
| testRegion := "us-west-2" | ||
| testClusterVpcId := "vpc-123456" | ||
| testClusterLocalGateway := "default" | ||
| | ||
| os.Setenv(REGION, testRegion) | ||
| os.Setenv(CLUSTER_VPC_ID, testClusterVpcId) | ||
| os.Setenv(CLUSTER_LOCAL_GATEWAY, testClusterLocalGateway) | ||
| os.Unsetenv(AWS_ACCOUNT_ID) | ||
| os.Unsetenv(TARGET_GROUP_NAME_LEN_MODE) | ||
| ConfigInit() | ||
| assert.Equal(t, Region, testRegion) | ||
| assert.Equal(t, VpcID, testClusterVpcId) | ||
| assert.Equal(t, AccountID, UnknownInput) | ||
| assert.Equal(t, DefaultServiceNetwork, testClusterLocalGateway) | ||
| assert.Equal(t, UseLongTGName, false) | ||
| } | ||
| | ||
| func Test_config_init_no_env_var(t *testing.T) { | ||
| os.Unsetenv(REGION) | ||
| os.Unsetenv(CLUSTER_VPC_ID) | ||
| os.Unsetenv(CLUSTER_LOCAL_GATEWAY) | ||
| os.Unsetenv(AWS_ACCOUNT_ID) | ||
| os.Unsetenv(TARGET_GROUP_NAME_LEN_MODE) | ||
| ConfigInit() | ||
| assert.Equal(t, Region, UnknownInput) | ||
| assert.Equal(t, VpcID, UnknownInput) | ||
| assert.Equal(t, AccountID, UnknownInput) | ||
| assert.Equal(t, DefaultServiceNetwork, UnknownInput) | ||
| assert.Equal(t, UseLongTGName, false) | ||
| } | ||
| | ||
| func Test_config_init_with_all_env_var(t *testing.T) { | ||
| // Test variable | ||
| testRegion := "us-west-2" | ||
| testClusterVpcId := "vpc-123456" | ||
| testClusterLocalGateway := "default" | ||
| testTargetGroupNameLenMode := "long" | ||
| testAwsAccountId := "12345678" | ||
| | ||
| os.Setenv(REGION, testRegion) | ||
| os.Setenv(CLUSTER_VPC_ID, testClusterVpcId) | ||
| os.Setenv(CLUSTER_LOCAL_GATEWAY, testClusterLocalGateway) | ||
| os.Setenv(AWS_ACCOUNT_ID, testAwsAccountId) | ||
| os.Setenv(TARGET_GROUP_NAME_LEN_MODE, testTargetGroupNameLenMode) | ||
| ConfigInit() | ||
| assert.Equal(t, Region, testRegion) | ||
| assert.Equal(t, VpcID, testClusterVpcId) | ||
| assert.Equal(t, AccountID, testAwsAccountId) | ||
| assert.Equal(t, DefaultServiceNetwork, testClusterLocalGateway) | ||
| assert.Equal(t, UseLongTGName, true) | ||
| } |
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.
can u also remove //TODO comment? thanks