-
Notifications
You must be signed in to change notification settings - Fork 71
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") | ||
|
Contributor
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)
Contributor
Author
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) | ||
| } | ||
|
Contributor
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.
Contributor
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
Contributor
Author
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