Revert "Enable metadata update from project attributes"
This reverts commit f0f71c74f7dd233ce27eefd9fe43b123e4876e9e.
Reason for revert: k8s tests failing after this change
Change-Id: I98d1615cca9c85f2941dc4e808b3754f396006fe
diff --git a/pkg/configfetcher/configfetcher.go b/pkg/configfetcher/configfetcher.go
index caddf7b..7a93e98 100644
--- a/pkg/configfetcher/configfetcher.go
+++ b/pkg/configfetcher/configfetcher.go
@@ -89,6 +89,22 @@
}
}
+// getUserConfig gets the content of the instance custom metadata keys'.
+// Then getUserConfig parses the metadata keys and returns userConfig.
+func getUserConfig() (*protos.InstanceConfig, error) {
+ var metadataKeys []string
+ var err error
+ if metadataKeys, err = metadata.InstanceAttributes(); err != nil {
+ return nil, fmt.Errorf("failed to fetch metadata from metadata server: %s", err)
+ }
+ glog.V(4).Infof("Current value of instance custom metadata keys: %v", metadataKeys)
+ userConfig, err := getUserConfigFromMetadata(metadataKeys)
+ if err != nil {
+ return nil, err
+ }
+ return userConfig, nil
+}
+
// GetInstanceID returns the id of the instance.
func GetInstanceID() (uint64, error) {
var resp, err = metadata.InstanceID()
@@ -103,50 +119,27 @@
// corresponding key with 'gci-' prefix. For example, 'cos-update-strategy' and
// 'gci-update-strategy'. The value of 'gci-' key is returned only if the function
// fail to get the value of 'cos-' key.
-func getCOSGCIConfigSetting(cosKey string, gciKey string) (string, bool) {
- if cosKey != "" {
- val, err := metadata.InstanceAttributeValue(cosKey)
- if err == nil {
- return val, true
- }
- if _, ok := err.(metadata.NotDefinedError); !ok {
- glog.Error(err)
- }
- }
- if gciKey != "" {
- val, err := metadata.InstanceAttributeValue(gciKey)
- if err == nil {
- return val, true
- }
- if _, ok := err.(metadata.NotDefinedError); !ok {
- glog.Error(err)
- }
- }
- if cosKey != "" {
- val, err := metadata.ProjectAttributeValue(cosKey)
- if err == nil {
- return val, true
- }
- if _, ok := err.(metadata.NotDefinedError); !ok {
- glog.Error(err)
- }
- }
- if gciKey != "" {
- val, err := metadata.ProjectAttributeValue(gciKey)
- if err == nil {
- return val, true
- }
- if _, ok := err.(metadata.NotDefinedError); !ok {
- glog.Error(err)
- }
- }
+func getCOSGCIConfigSetting(cosKey string, gciKey string, metadataKeys []string) (string, bool) {
+ var val string
+ var err error
+ for _, metadataKey := range metadataKeys {
+ if metadataKey == cosKey {
+ if val, err = metadata.InstanceAttributeValue(cosKey); err == nil {
+ return val, true
+ }
+ } else if metadataKey == gciKey {
+ if val, err = metadata.InstanceAttributeValue(gciKey); err == nil {
+ return val, true
+ }
+ }
+ }
return "", false
}
// Gets user config based on the metadata keys. An empty InstanceConfig is returned
// if there is no InstanceConfig-related metadata in the pairs.
-func getUserConfig() (*protos.InstanceConfig, error) {
+func getUserConfigFromMetadata(metadataKeys []string) (*protos.InstanceConfig, error) {
var configStr string
var err error
var ok, boolean bool
@@ -155,7 +148,7 @@
// Get legacy instance config from metadata. This is deprecated, but if this is still
// being specified, use it and ignore others.
- if configStr, ok = getCOSGCIConfigSetting("", gciLegacyConfigKey); ok {
+ if configStr, ok = getCOSGCIConfigSetting("", gciLegacyConfigKey, metadataKeys); ok {
if configStr != "" {
if err := jsonpb.UnmarshalString(configStr, userConfig); err != nil {
return nil, fmt.Errorf("failed to unmarshal InstanceConfig %s: %s", configStr, err)
@@ -165,7 +158,7 @@
}
// Get individual config settings. Keys with 'cos-' prefix have the priority.
- if configStr, ok = getCOSGCIConfigSetting(cosKeyUpdateStrategy, gciKeyUpdateStrategy); ok {
+ if configStr, ok = getCOSGCIConfigSetting(cosKeyUpdateStrategy, gciKeyUpdateStrategy, metadataKeys); ok {
if configStr == "update_disabled" {
userConfig.UpdateStrategy = proto.String(configStr)
} else {
@@ -173,19 +166,19 @@
}
}
- if configStr, ok = getCOSGCIConfigSetting(cosKeyMetricsEnabled, gciKeyMetricsEnabled); ok {
+ if configStr, ok = getCOSGCIConfigSetting(cosKeyMetricsEnabled, gciKeyMetricsEnabled, metadataKeys); ok {
if boolean, err = strconv.ParseBool(configStr); err == nil {
userConfig.MetricsEnabled = proto.Bool(boolean)
}
}
- if configStr, ok := getCOSGCIConfigSetting(keyGoogleLoggingEnabled, ""); ok {
+ if configStr, ok := getCOSGCIConfigSetting(keyGoogleLoggingEnabled, "", metadataKeys); ok {
if boolean, err = strconv.ParseBool(configStr); err == nil {
userConfig.HealthMonitorConfig.LoggingEnabled = proto.Bool(boolean)
}
}
- if configStr, ok := getCOSGCIConfigSetting(keyGoogleMonitoringEnabled, ""); ok {
+ if configStr, ok := getCOSGCIConfigSetting(keyGoogleMonitoringEnabled, "", metadataKeys); ok {
if boolean, err = strconv.ParseBool(configStr); err == nil {
userConfig.HealthMonitorConfig.MonitoringEnabled = proto.Bool(boolean)
}
diff --git a/pkg/configfetcher/configfetcher_test.go b/pkg/configfetcher/configfetcher_test.go
index 817a79a..dcd70e8 100644
--- a/pkg/configfetcher/configfetcher_test.go
+++ b/pkg/configfetcher/configfetcher_test.go
@@ -22,34 +22,37 @@
"policy-manager/protos"
)
-// Tests that getUserConfig() calls metadata client and
-// returns correct config settings.
-func TestGetUserConfig(t *testing.T) {
+// Tests that all metadata keys are parsed and applied correctly.
+func TestGetUserConfigFromMetadata(t *testing.T) {
tests := []struct {
// Name of the test case
name string
- // The instanceMetadata returned by metatdata client
- instanceMetadata map[string]string
- // The projectMetadata returned by metatdata client
- projectMetadata map[string]string
- // The expected config settings to be returned
+ // Key:Value pairs present in metadata.
+ metadata map[string]string
+ // Expected InstanceConfig to be returned
expectedConfig *protos.InstanceConfig
}{
{
- "NoMetadata",
- map[string]string{},
+ "NoMetadataKeys",
map[string]string{},
&protos.InstanceConfig{
HealthMonitorConfig: &protos.HealthMonitorConfig{},
},
},
{
- "InstanceUpdateOnly",
- map[string]string{
- "cos-metrics-enabled": "true",
- "cos-update-strategy": "",
+ "UpdateStrategyPresent",
+ map[string]string{gciKeyUpdateStrategy: "update-strategy-1"},
+ &protos.InstanceConfig{
+ UpdateStrategy: proto.String(""),
+ HealthMonitorConfig: &protos.HealthMonitorConfig{},
},
- map[string]string{},
+ },
+ {
+ "MultipleSettingsPresent",
+ map[string]string{
+ gciKeyUpdateStrategy: "update-strategy-2",
+ gciKeyMetricsEnabled: "true",
+ },
&protos.InstanceConfig{
UpdateStrategy: proto.String(""),
MetricsEnabled: proto.Bool(true),
@@ -57,8 +60,118 @@
},
},
{
- "ProjectUpdateOnly",
+ "InvalidBoolValueIgnored",
+ map[string]string{
+ gciKeyMetricsEnabled: "no",
+ keyGoogleLoggingEnabled: "yes",
+ },
+ &protos.InstanceConfig{
+ HealthMonitorConfig: &protos.HealthMonitorConfig{},
+ },
+ },
+ {
+ "LegacyConfigKeyPresent",
+ map[string]string{
+ gciLegacyConfigKey: "{\"update_strategy\":\"update-strategy-3\"}",
+ gciKeyUpdateStrategy: "update-strategy-4",
+ },
+ &protos.InstanceConfig{
+ UpdateStrategy: proto.String("update-strategy-3"),
+ HealthMonitorConfig: &protos.HealthMonitorConfig{},
+ },
+ },
+ {
+ "LoggingButIngoreMonitoring",
+ map[string]string{
+ keyGoogleLoggingEnabled: "true",
+ },
+ &protos.InstanceConfig{
+ HealthMonitorConfig: &protos.HealthMonitorConfig{
+ Enforced: proto.Bool(true),
+ LoggingEnabled: proto.Bool(true),
+ },
+ },
+ },
+ {
+ "MonitoringButIngoreLogging",
+ map[string]string{
+ keyGoogleMonitoringEnabled: "true",
+ },
+ &protos.InstanceConfig{
+ HealthMonitorConfig: &protos.HealthMonitorConfig{
+ Enforced: proto.Bool(true),
+ MonitoringEnabled: proto.Bool(true),
+ },
+ },
+ },
+ {
+ "SpecifiedNoLogging",
+ map[string]string{
+ keyGoogleLoggingEnabled: "false",
+ },
+ &protos.InstanceConfig{
+ HealthMonitorConfig: &protos.HealthMonitorConfig{
+ LoggingEnabled: proto.Bool(false),
+ },
+ },
+ },
+ {
+ "LoggingAndMonitoring",
+ map[string]string{
+ keyGoogleMonitoringEnabled: "true",
+ keyGoogleLoggingEnabled: "true",
+ },
+ &protos.InstanceConfig{
+ HealthMonitorConfig: &protos.HealthMonitorConfig{
+ Enforced: proto.Bool(true),
+ LoggingEnabled: proto.Bool(true),
+ MonitoringEnabled: proto.Bool(true),
+ },
+ },
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ fakeMetadata := fakes.NewMetadataServer()
+ defer fakeMetadata.Server.Close()
+ metadataKeys := make([]string, 0, len(test.metadata))
+ for k := range test.metadata {
+ metadataKeys = append(metadataKeys, k)
+ }
+
+ fakeMetadata.Metadata = test.metadata
+ actualConfig, _ := getUserConfigFromMetadata(metadataKeys)
+ if !proto.Equal(actualConfig, test.expectedConfig) {
+ t.Errorf("FAILED '%s': got %s, expect %s",
+ test.name,
+ proto.MarshalTextString(actualConfig),
+ proto.MarshalTextString(test.expectedConfig))
+ }
+ })
+ }
+}
+
+// Tests that getUserConfig() calls metadata client and
+// returns correct config settings.
+func TestGetUserConfig(t *testing.T) {
+ tests := []struct {
+ // Name of the test case
+ name string
+ // The metadata returned by metatdata client
+ metadata map[string]string
+ // The expected config settings to be returned
+ expectedConfig *protos.InstanceConfig
+ }{
+ {
+ "NoMetadata",
map[string]string{},
+ &protos.InstanceConfig{
+ HealthMonitorConfig: &protos.HealthMonitorConfig{},
+ },
+ },
+ {
+ "NormalUpdate",
map[string]string{
"cos-metrics-enabled": "true",
"cos-update-strategy": "",
@@ -76,23 +189,6 @@
"key1": "value1",
"kdy2": "value2",
},
- map[string]string{
- "cos-metrics-enabled": "true",
- },
- &protos.InstanceConfig{
- UpdateStrategy: proto.String("update_disabled"),
- MetricsEnabled: proto.Bool(true),
- HealthMonitorConfig: &protos.HealthMonitorConfig{},
- },
- },
- {
- "SameMetadataTag",
- map[string]string{
- "cos-update-strategy": "update_disabled",
- },
- map[string]string{
- "cos-update-strategy": "",
- },
&protos.InstanceConfig{
UpdateStrategy: proto.String("update_disabled"),
HealthMonitorConfig: &protos.HealthMonitorConfig{},
@@ -104,9 +200,6 @@
"key1": "value1",
"kdy2": "value2",
},
- map[string]string{
- "key1": "value1",
- },
&protos.InstanceConfig{
HealthMonitorConfig: &protos.HealthMonitorConfig{},
},
@@ -119,7 +212,6 @@
"key1": "value1",
"kdy2": "value2",
},
- map[string]string{},
&protos.InstanceConfig{
HealthMonitorConfig: &protos.HealthMonitorConfig{
Enforced: proto.Bool(true),
@@ -129,10 +221,11 @@
},
},
{
- "MonitoringButIgnoreLoggingAtProjectLevel",
- map[string]string{},
+ "MonitoringButIgnoreLogging",
map[string]string{
"google-monitoring-enabled": "true",
+ "key1": "value1",
+ "kdy2": "value2",
},
&protos.InstanceConfig{
HealthMonitorConfig: &protos.HealthMonitorConfig{
@@ -142,18 +235,17 @@
},
},
{
- "LoggingAndMonitoringAtProjectLevelButNotAtInstanceLevel",
+ "LoggingAndMonitoring",
map[string]string{
- "google-logging-enabled": "false",
+ "google-logging-enabled": "true",
"google-monitoring-enabled": "true",
- },
- map[string]string{
- "google-logging-enabled": "true",
+ "key1": "value1",
+ "kdy2": "value2",
},
&protos.InstanceConfig{
HealthMonitorConfig: &protos.HealthMonitorConfig{
Enforced: proto.Bool(true),
- LoggingEnabled: proto.Bool(false),
+ LoggingEnabled: proto.Bool(true),
MonitoringEnabled: proto.Bool(true),
},
},
@@ -164,14 +256,12 @@
t.Run(test.name, func(t *testing.T) {
fakeMetadata := fakes.NewMetadataServer()
defer fakeMetadata.Server.Close()
- fakeMetadata.InstanceMetadata = test.instanceMetadata
- fakeMetadata.ProjectMetadata = test.projectMetadata
- actualConfig, err := getUserConfig()
- if err != nil {
- t.Fatal(err)
- }
+ fakeMetadata.Metadata = test.metadata
+ actualConfig, _ := getUserConfig()
+
if !proto.Equal(actualConfig, test.expectedConfig) {
- t.Errorf("got %s, expect %s",
+ t.Errorf("FAILED '%s': got %s, expect %s",
+ test.name,
proto.MarshalTextString(actualConfig),
proto.MarshalTextString(test.expectedConfig))
}
diff --git a/pkg/fakes/metadata.go b/pkg/fakes/metadata.go
index 656f5a1..e1d0186 100644
--- a/pkg/fakes/metadata.go
+++ b/pkg/fakes/metadata.go
@@ -25,24 +25,21 @@
// GCEMetadata is a fake GCE implementation. It is intended to be constructed with NewMetadataServer.
type GCEMetadata struct {
- // InstanceMetadata represents the collection of objects that exist in the fake GCS server.
+ // MetadataKeys represents the collection of objects that exist in the fake GCS server.
// Keys and values are strings of the form "/instance/attributes".
- InstanceMetadata map[string]string
- // ProjectMetadata represents the collection of objects that exist in the fake GCS server.
- // Keys and values are strings of the form "/project/attributes".
- ProjectMetadata map[string]string
+ Metadata map[string]string
// Server is an HTTP server that serves fake metadata requests. Requests are served using the state stored in
// the other struct fields.
Server *httptest.Server
}
-// instanceMetadataKeysHandler writes all metadata keys or values to a particular metadata key upon request
-func (mtd *GCEMetadata) instanceMetadataKeysHandler(w http.ResponseWriter, r *http.Request) {
- instanceMetadataKey := strings.TrimPrefix(r.URL.Path, "/computeMetadata/v1/instance/attributes/")
+// metadataKeysHandler writes all metadata keys or values to a particular metadata key upon request
+func (mtd *GCEMetadata) metadataKeysHandler(w http.ResponseWriter, r *http.Request) {
+ metadataKey := strings.TrimPrefix(r.URL.Path, "/computeMetadata/v1/instance/attributes/")
- if instanceMetadataKey == "" {
+ if metadataKey == "" {
var res string
- for k := range mtd.InstanceMetadata {
+ for k := range mtd.Metadata {
res += strings.TrimSpace(k) + "\n"
}
if _, err := w.Write([]byte(res)); err != nil {
@@ -50,39 +47,10 @@
}
} else {
- if val, ok := mtd.InstanceMetadata[instanceMetadataKey]; ok {
- res := string(val)
- if _, err := w.Write([]byte(res)); err != nil {
- log.Printf("write %q failed: %v", r.URL.Path, err)
- }
- } else {
- http.Error(w, "Key not found", 404)
- }
- }
-}
-
-// metadataprojectMetadataKeysHandlerKeysHandler writes all metadata keys or values to a particular metadata key upon request
-func (mtd *GCEMetadata) projectMetadataKeysHandler(w http.ResponseWriter, r *http.Request) {
- projectMetadataKey := strings.TrimPrefix(r.URL.Path, "/computeMetadata/v1/project/attributes/")
-
- if projectMetadataKey == "" {
- var res string
- for k := range mtd.ProjectMetadata {
- res += strings.TrimSpace(k) + "\n"
- }
+ res := string(mtd.Metadata[metadataKey])
if _, err := w.Write([]byte(res)); err != nil {
log.Printf("write %q failed: %v", r.URL.Path, err)
}
-
- } else {
- if val, ok := mtd.ProjectMetadata[projectMetadataKey]; ok {
- res := string(val)
- if _, err := w.Write([]byte(res)); err != nil {
- log.Printf("write %q failed: %v", r.URL.Path, err)
- }
- } else {
- http.Error(w, "Key not found", 404)
- }
}
}
@@ -91,14 +59,12 @@
// use of this environment variable makes it unsafe for concurrency.
func NewMetadataServer() *GCEMetadata {
mtd := &GCEMetadata{
- InstanceMetadata: make(map[string]string),
- ProjectMetadata: make(map[string]string),
- Server: nil,
+ Metadata: make(map[string]string),
+ Server: nil,
}
mux := http.NewServeMux()
- mux.HandleFunc("/computeMetadata/v1/instance/attributes/", mtd.instanceMetadataKeysHandler)
- mux.HandleFunc("/computeMetadata/v1/project/attributes/", mtd.projectMetadataKeysHandler)
+ mux.HandleFunc("/computeMetadata/v1/instance/attributes/", mtd.metadataKeysHandler)
mtd.Server = httptest.NewServer(mux)
// Setting metadata host url by skipping the http:// part
diff --git a/pkg/fakes/metadata_test.go b/pkg/fakes/metadata_test.go
index 5a93d46..06640fa 100644
--- a/pkg/fakes/metadata_test.go
+++ b/pkg/fakes/metadata_test.go
@@ -16,11 +16,12 @@
import (
"cloud.google.com/go/compute/metadata"
+ "fmt"
"testing"
)
// Tests that all metadata keys are parsed and applied correctly.
-func TestInstanceMetadataKeysHandler(t *testing.T) {
+func TestMetadataKeysHandler(t *testing.T) {
tests := []struct {
// Name of the test case
name string
@@ -36,17 +37,13 @@
},
{
"UpdateStrategyPresent",
- map[string]string{
- "update-strategy": "update-strategy-1",
- },
+ map[string]string{"update-strategy": "update-strategy-1"},
[]string{"update-strategy"},
},
{
"TwoKeysPresent",
- map[string]string{
- "cos-update-strategy": "update-strategy-1",
- "google-logging-enabled": "true",
- },
+ map[string]string{"cos-update-strategy": "update-strategy-1",
+ "google-logging-enabled": "true"},
[]string{"cos-update-strategy", "google-logging-enabled"},
},
}
@@ -54,64 +51,17 @@
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
fakeMetadata := NewMetadataServer()
- fakeMetadata.InstanceMetadata = test.metadata
+ fakeMetadata.Metadata = test.metadata
defer fakeMetadata.Server.Close()
if res, _ := metadata.InstanceAttributes(); len(res) != len(test.expectedMetadataKeys) {
- t.Errorf("got %v, expect %v", res, test.expectedMetadataKeys)
+ fmt.Printf("%v", test.expectedMetadataKeys)
+ t.Errorf("FAILED '%s': got %v, expect %v",
+ test.name, res, test.expectedMetadataKeys)
} else {
for _, val := range res {
if metadataVal, _ := metadata.InstanceAttributeValue(val); metadataVal != test.metadata[val] {
- t.Errorf("got %v, expect %v", metadataVal, test.metadata[val])
- }
- }
- }
- })
- }
-}
-
-// Tests that all metadata keys are parsed and applied correctly.
-func TestProjectMetadataKeysHandler(t *testing.T) {
- tests := []struct {
- // Name of the test case
- name string
- // Key:Value pairs present in metadata.
- metadata map[string]string
- // Expected metadataKeys to be returned
- expectedMetadataKeys []string
- }{
- {
- "NoMetadataKeys",
- map[string]string{},
- []string{""},
- },
- {
- "UpdateStrategyPresent",
- map[string]string{
- "update-strategy": "update-strategy-1",
- },
- []string{"update-strategy"},
- },
- {
- "TwoKeysPresent",
- map[string]string{
- "cos-update-strategy": "update-strategy-1",
- "google-logging-enabled": "true",
- },
- []string{"cos-update-strategy", "google-logging-enabled"},
- },
- }
-
- for _, test := range tests {
- t.Run(test.name, func(t *testing.T) {
- fakeMetadata := NewMetadataServer()
- fakeMetadata.ProjectMetadata = test.metadata
- defer fakeMetadata.Server.Close()
- if res, _ := metadata.ProjectAttributes(); len(res) != len(test.expectedMetadataKeys) {
- t.Errorf("got %v, expect %v", res, test.expectedMetadataKeys)
- } else {
- for _, val := range res {
- if metadataVal, _ := metadata.ProjectAttributeValue(val); metadataVal != test.metadata[val] {
- t.Errorf("got %v, expect %v", metadataVal, test.metadata[val])
+ t.Errorf("FAILED '%s': got %v, expect %v",
+ test.name, metadataVal, test.metadata[val])
}
}
}
