From 1111964fee74bfaeed34437a7a54b69fade8cd2a Mon Sep 17 00:00:00 2001 From: Murat Kabilov Date: Fri, 26 May 2017 18:13:48 +0200 Subject: [PATCH 1/3] fix password check in pguserpassword remove magic number --- pkg/util/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/util/util.go b/pkg/util/util.go index 7f38b9a7e..e67a02294 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -41,7 +41,7 @@ func NameFromMeta(meta v1.ObjectMeta) spec.NamespacedName { } func PGUserPassword(user spec.PgUser) string { - if (len(user.Password) == md5.Size && user.Password[:3] == md5prefix) || user.Password == "" { + if (len(user.Password) == md5.Size*2+len(md5prefix) && user.Password[:3] == md5prefix) || user.Password == "" { // Avoid processing already encrypted or empty passwords return user.Password } From f7aaf8863de5c2cf6ad2418b888720119f03e8b3 Mon Sep 17 00:00:00 2001 From: Murat Kabilov Date: Tue, 30 May 2017 09:56:10 +0200 Subject: [PATCH 2/3] Change maintenance window format --- manifests/testpostgresql.yaml | 2 +- pkg/spec/postgresql.go | 112 ++++++++++++++++++---------------- 2 files changed, 61 insertions(+), 53 deletions(-) diff --git a/manifests/testpostgresql.yaml b/manifests/testpostgresql.yaml index 44f1fad78..1728f46a7 100644 --- a/manifests/testpostgresql.yaml +++ b/manifests/testpostgresql.yaml @@ -43,4 +43,4 @@ spec: maximum_lag_on_failover: 33554432 maintenanceWindows: - 01:00-06:00 #UTC - - Sat:00:00-Sat:04:00 + - Sat:00:00-04:00 diff --git a/pkg/spec/postgresql.go b/pkg/spec/postgresql.go index e00625c7d..0ff3d9ff3 100644 --- a/pkg/spec/postgresql.go +++ b/pkg/spec/postgresql.go @@ -3,7 +3,6 @@ package spec import ( "encoding/json" "fmt" - "regexp" "strings" "time" @@ -12,14 +11,11 @@ import ( "k8s.io/client-go/pkg/api/v1" ) -var alphaRegexp = regexp.MustCompile("^[a-zA-Z]*$") - type MaintenanceWindow struct { - StartTime time.Time // Start time - StartWeekday time.Weekday // Start weekday - - EndTime time.Time // End time - EndWeekday time.Weekday // End weekday + Everyday bool + Weekday time.Weekday + StartTime time.Time // Start time + EndTime time.Time // End time } type Volume struct { @@ -71,7 +67,7 @@ type Postgresql struct { Metadata v1.ObjectMeta `json:"metadata"` Spec PostgresSpec `json:"spec"` - Status PostgresStatus `json:"status"` + Status PostgresStatus `json:"status,omitempty"` Error error `json:"-"` } @@ -96,54 +92,49 @@ type PostgresqlList struct { Items []Postgresql `json:"items"` } -func parseTime(s string) (t time.Time, wd time.Weekday, wdProvided bool, err error) { - var timeLayout string +var weekdays = map[string]int{"Sun": 0, "Mon": 1, "Tue": 2, "Wed": 3, "Thu": 4, "Fri": 5, "Sat": 6} +func parseTime(s string) (time.Time, error) { parts := strings.Split(s, ":") - if len(parts) == 3 { - if len(parts[0]) != 3 || !alphaRegexp.MatchString(parts[0]) { - err = fmt.Errorf("weekday must be 3 characters length") - return - } - timeLayout = "Mon:15:04" - wdProvided = true - } else { - wdProvided = false - timeLayout = "15:04" + if len(parts) != 2 { + return time.Time{}, fmt.Errorf("incorrect time format") } + timeLayout := "15:04" tp, err := time.Parse(timeLayout, s) if err != nil { - return + return time.Time{}, err } - wd = tp.Weekday() - t = tp.UTC() + return tp.UTC(), nil +} - return +func parseWeekday(s string) (time.Weekday, error) { + weekday, ok := weekdays[s] + if !ok { + return time.Weekday(0), fmt.Errorf("incorrect weekday") + } + + return time.Weekday(weekday), nil } func (m *MaintenanceWindow) MarshalJSON() ([]byte, error) { - var startWd, endWd string - if m.StartWeekday == time.Sunday && m.EndWeekday == time.Saturday { - startWd = "" - endWd = "" + if m.Everyday { + return []byte(fmt.Sprintf("\"%s-%s\"", + m.StartTime.Format("15:04"), + m.EndTime.Format("15:04"))), nil } else { - startWd = m.StartWeekday.String()[:3] + ":" - endWd = m.EndWeekday.String()[:3] + ":" + return []byte(fmt.Sprintf("\"%s:%s-%s\"", + m.Weekday.String()[:3], + m.StartTime.Format("15:04"), + m.EndTime.Format("15:04"))), nil } - - return []byte(fmt.Sprintf("\"%s%s-%s%s\"", - startWd, m.StartTime.Format("15:04"), - endWd, m.EndTime.Format("15:04"))), nil } func (m *MaintenanceWindow) UnmarshalJSON(data []byte) error { var ( - got MaintenanceWindow - weekdayProvidedFrom bool - weekdayProvidedTo bool - err error + got MaintenanceWindow + err error ) parts := strings.Split(string(data[1:len(data)-1]), "-") @@ -151,25 +142,35 @@ func (m *MaintenanceWindow) UnmarshalJSON(data []byte) error { return fmt.Errorf("incorrect maintenance window format") } - got.StartTime, got.StartWeekday, weekdayProvidedFrom, err = parseTime(parts[0]) + fromParts := strings.Split(parts[0], ":") + switch len(fromParts) { + case 3: + got.Everyday = false + got.Weekday, err = parseWeekday(fromParts[0]) + if err != nil { + return fmt.Errorf("could not parse weekday: %v", err) + } + + got.StartTime, err = parseTime(fromParts[1] + ":" + fromParts[2]) + case 2: + got.Everyday = true + got.StartTime, err = parseTime(fromParts[0] + ":" + fromParts[1]) + default: + return fmt.Errorf("incorrect maintenance window format") + } if err != nil { - return err + return fmt.Errorf("could not parse start time: %v", err) } - got.EndTime, got.EndWeekday, weekdayProvidedTo, err = parseTime(parts[1]) + got.EndTime, err = parseTime(parts[1]) if err != nil { - return err + return fmt.Errorf("could not parse end time: %v", err) } if got.EndTime.Before(got.StartTime) { return fmt.Errorf("'From' time must be prior to the 'To' time") } - if !weekdayProvidedFrom || !weekdayProvidedTo { - got.StartWeekday = time.Sunday - got.EndWeekday = time.Saturday - } - *m = got return nil @@ -191,11 +192,16 @@ func (pl *PostgresqlList) GetListMeta() unversioned.List { return &pl.Metadata } -func clusterName(clusterName string, teamName string) (string, error) { +func extractClusterName(clusterName string, teamName string) (string, error) { teamNameLen := len(teamName) if len(clusterName) < teamNameLen+2 { return "", fmt.Errorf("name is too short") } + + if teamNameLen == 0 { + return "", fmt.Errorf("team name is empty") + } + if strings.ToLower(clusterName[:teamNameLen+1]) != strings.ToLower(teamName)+"-" { return "", fmt.Errorf("name must match {TEAM}-{NAME} format") } @@ -211,7 +217,8 @@ type PostgresqlListCopy PostgresqlList type PostgresqlCopy Postgresql func (p *Postgresql) UnmarshalJSON(data []byte) error { - tmp := PostgresqlCopy{} + var tmp PostgresqlCopy + err := json.Unmarshal(data, &tmp) if err != nil { metaErr := json.Unmarshal(data, &tmp.Metadata) @@ -228,7 +235,7 @@ func (p *Postgresql) UnmarshalJSON(data []byte) error { } tmp2 := Postgresql(tmp) - clusterName, err := clusterName(tmp2.Metadata.Name, tmp2.Spec.TeamID) + clusterName, err := extractClusterName(tmp2.Metadata.Name, tmp2.Spec.TeamID) if err == nil { tmp2.Spec.ClusterName = clusterName } else { @@ -241,7 +248,8 @@ func (p *Postgresql) UnmarshalJSON(data []byte) error { } func (pl *PostgresqlList) UnmarshalJSON(data []byte) error { - tmp := PostgresqlListCopy{} + var tmp PostgresqlListCopy + err := json.Unmarshal(data, &tmp) if err != nil { return err From 1fb05212a9cd37409f847ead1d0498571313e641 Mon Sep 17 00:00:00 2001 From: Murat Kabilov Date: Tue, 30 May 2017 10:14:30 +0200 Subject: [PATCH 3/3] Refactor teams API package --- pkg/cluster/util.go | 32 +++++- pkg/controller/controller.go | 4 +- pkg/controller/util.go | 19 ---- pkg/util/teams/teams.go | 26 ++--- pkg/util/teams/teams_test.go | 182 +++++++++++++++++++++++++++++++++++ 5 files changed, 222 insertions(+), 41 deletions(-) create mode 100644 pkg/util/teams/teams_test.go diff --git a/pkg/cluster/util.go b/pkg/cluster/util.go index c8a3ee18d..2302e1d47 100644 --- a/pkg/cluster/util.go +++ b/pkg/cluster/util.go @@ -105,11 +105,41 @@ func (c *Cluster) logVolumeChanges(old, new spec.Volume, reason string) { } } +func (c *Cluster) getOAuthToken() (string, error) { + //TODO: we can move this function to the Controller in case it will be needed there. As for now we use it only in the Cluster + // Temporary getting postgresql-operator secret from the NamespaceDefault + credentialsSecret, err := c.KubeClient. + Secrets(c.OpConfig.OAuthTokenSecretName.Namespace). + Get(c.OpConfig.OAuthTokenSecretName.Name) + + if err != nil { + c.logger.Debugf("Oauth token secret name: %s", c.OpConfig.OAuthTokenSecretName) + return "", fmt.Errorf("could not get credentials secret: %v", err) + } + data := credentialsSecret.Data + + if string(data["read-only-token-type"]) != "Bearer" { + return "", fmt.Errorf("wrong token type: %v", data["read-only-token-type"]) + } + + return string(data["read-only-token-secret"]), nil +} + func (c *Cluster) getTeamMembers() ([]string, error) { if c.Spec.TeamID == "" { return nil, fmt.Errorf("no teamId specified") } - teamInfo, err := c.TeamsAPIClient.TeamInfo(c.Spec.TeamID) + if !c.OpConfig.EnableTeamsAPI { + c.logger.Debug("Team API is disabled, returning empty list of members") + return []string{}, nil + } + + token, err := c.getOAuthToken() + if err != nil { + return []string{}, fmt.Errorf("could not get oauth token: %v", err) + } + + teamInfo, err := c.TeamsAPIClient.TeamInfo(c.Spec.TeamID, token) if err != nil { return nil, fmt.Errorf("could not get team info: %v", err) } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 0f74774ae..b4801d28c 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -47,7 +47,8 @@ func New(controllerConfig *Config, operatorConfig *config.Config) *Controller { logger.Level = logrus.DebugLevel } - controllerConfig.TeamsAPIClient = teams.NewTeamsAPI(operatorConfig.TeamsAPIUrl, logger, operatorConfig.EnableTeamsAPI) + controllerConfig.TeamsAPIClient = teams.NewTeamsAPI(operatorConfig.TeamsAPIUrl, logger) + return &Controller{ Config: *controllerConfig, opConfig: operatorConfig, @@ -78,7 +79,6 @@ func (c *Controller) initController() { c.logger.Fatalf("could not register ThirdPartyResource: %v", err) } - c.TeamsAPIClient.RefreshTokenAction = c.getOAuthToken if infraRoles, err := c.getInfrastructureRoles(); err != nil { c.logger.Warningf("could not get infrastructure roles: %v", err) } else { diff --git a/pkg/controller/util.go b/pkg/controller/util.go index f33f7ca7e..fcf15ec06 100644 --- a/pkg/controller/util.go +++ b/pkg/controller/util.go @@ -29,25 +29,6 @@ func (c *Controller) makeClusterConfig() cluster.Config { } } -func (c *Controller) getOAuthToken() (string, error) { - // Temporary getting postgresql-operator secret from the NamespaceDefault - credentialsSecret, err := c.KubeClient. - Secrets(c.opConfig.OAuthTokenSecretName.Namespace). - Get(c.opConfig.OAuthTokenSecretName.Name) - - if err != nil { - c.logger.Debugf("Oauth token secret name: %s", c.opConfig.OAuthTokenSecretName) - return "", fmt.Errorf("could not get credentials secret: %v", err) - } - data := credentialsSecret.Data - - if string(data["read-only-token-type"]) != "Bearer" { - return "", fmt.Errorf("wrong token type: %v", data["read-only-token-type"]) - } - - return string(data["read-only-token-secret"]), nil -} - func thirdPartyResource(TPRName string) *extv1beta.ThirdPartyResource { return &extv1beta.ThirdPartyResource{ ObjectMeta: v1.ObjectMeta{ diff --git a/pkg/util/teams/teams.go b/pkg/util/teams/teams.go index 993db8178..e2309065e 100644 --- a/pkg/util/teams/teams.go +++ b/pkg/util/teams/teams.go @@ -38,34 +38,22 @@ type Team struct { } type API struct { - url string - httpClient *http.Client - logger *logrus.Entry - RefreshTokenAction func() (string, error) - enabled bool + url string + httpClient *http.Client + logger *logrus.Entry } -func NewTeamsAPI(url string, log *logrus.Logger, enabled bool) *API { +func NewTeamsAPI(url string, log *logrus.Logger) *API { t := API{ url: strings.TrimRight(url, "/"), httpClient: &http.Client{}, logger: log.WithField("pkg", "teamsapi"), - enabled: enabled, } return &t } -func (t *API) TeamInfo(teamID string) (*Team, error) { - // TODO: avoid getting a new token on every call to the Teams API. - if !t.enabled { - t.logger.Debug("Team API is disabled, returning empty list of members") - return &Team{}, nil - } - token, err := t.RefreshTokenAction() - if err != nil { - return nil, err - } +func (t *API) TeamInfo(teamID, token string) (*Team, error) { url := fmt.Sprintf("%s/teams/%s", t.url, teamID) t.logger.Debugf("Request url: %s", url) req, err := http.NewRequest("GET", url, nil) @@ -84,7 +72,7 @@ func (t *API) TeamInfo(teamID string) (*Team, error) { d := json.NewDecoder(resp.Body) err = d.Decode(&raw) if err != nil { - return nil, err + return nil, fmt.Errorf("team API query failed with status code %d and malformed response: %v", resp.StatusCode, err) } if errMessage, ok := raw["error"]; ok { @@ -97,7 +85,7 @@ func (t *API) TeamInfo(teamID string) (*Team, error) { d := json.NewDecoder(resp.Body) err = d.Decode(teamInfo) if err != nil { - return nil, err + return nil, fmt.Errorf("could not parse team API response: %v", err) } return teamInfo, nil diff --git a/pkg/util/teams/teams_test.go b/pkg/util/teams/teams_test.go new file mode 100644 index 000000000..0b95524fb --- /dev/null +++ b/pkg/util/teams/teams_test.go @@ -0,0 +1,182 @@ +package teams + +import ( + "fmt" + "github.com/Sirupsen/logrus" + "net/http" + "net/http/httptest" + "reflect" + "testing" +) + +var ( + logger = logrus.New() + token = "ec45b1cfbe7100c6315d183a3eb6cec0M2U1LWJkMzEtZDgzNzNmZGQyNGM3IiwiYXV0aF90aW1lIjoxNDkzNzMwNzQ1LCJpc3MiOiJodHRwcz" +) + +var teamsAPItc = []struct { + in string + inCode int + out *Team + err error +}{ + {`{ +"dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", +"id": "acid", +"id_name": "ACID", +"team_id": "111222", +"type": "official", +"name": "Acid team name", +"mail": [ +"email1@example.com", +"email2@example.com" +], +"alias": [ +"acid" +], +"member": [ + "member1", + "member2", + "member3" +], +"infrastructure-accounts": [ +{ + "id": "1234512345", + "name": "acid", + "provider": "aws", + "type": "aws", + "description": "", + "owner": "acid", + "owner_dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", + "disabled": false +}, +{ + "id": "5432154321", + "name": "db", + "provider": "aws", + "type": "aws", + "description": "", + "owner": "acid", + "owner_dn": "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", + "disabled": false +} +], +"cost_center": "00099999", +"delivery_lead": "member4", +"parent_team_id": "111221" +}`, + 200, + &Team{ + Dn: "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", + ID: "acid", + TeamName: "ACID", + TeamID: "111222", + Type: "official", + FullName: "Acid team name", + Aliases: []string{"acid"}, + Mails: []string{"email1@example.com", "email2@example.com"}, + Members: []string{"member1", "member2", "member3"}, + CostCenter: "00099999", + DeliveryLead: "member4", + ParentTeamID: "111221", + InfrastructureAccounts: []InfrastructureAccount{ + { + ID: "1234512345", + Name: "acid", + Provider: "aws", + Type: "aws", + Description: "", + Owner: "acid", + OwnerDn: "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", + Disabled: false}, + { + ID: "5432154321", + Name: "db", + Provider: "aws", + Type: "aws", + Description: "", + Owner: "acid", + OwnerDn: "cn=100100,ou=official,ou=foobar,dc=zalando,dc=net", + Disabled: false}, + }, + }, + nil}, { + `{"error": "Access Token not valid"}`, + 401, + nil, + fmt.Errorf(`team API query failed with status code 401 and message: '"Access Token not valid"'`), + }, + { + `{"status": "I'm a teapot'"}`, + 418, + nil, + fmt.Errorf(`team API query failed with status code 418`), + }, + { + `{"status": "I'm a teapot`, + 418, + nil, + fmt.Errorf(`team API query failed with status code 418 and malformed response: unexpected EOF`), + }, + { + `{"status": "I'm a teapot`, + 200, + nil, + fmt.Errorf(`could not parse team API response: unexpected EOF`), + }, +} + +var requestsURLtc = []struct { + url string + err error +}{ + { + "coffee://localhost/", + fmt.Errorf(`Get coffee://localhost/teams/acid: unsupported protocol scheme "coffee"`), + }, + { + "http://192.168.0.%31/", + fmt.Errorf(`parse http://192.168.0.%%31/teams/acid: invalid URL escape "%%31"`), + }, +} + +func TestInfo(t *testing.T) { + for _, tc := range teamsAPItc { + func() { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer " + token { + t.Errorf("Authorization token is wrong or not provided") + } + w.WriteHeader(tc.inCode) + fmt.Fprint(w, tc.in) + })) + defer ts.Close() + api := NewTeamsAPI(ts.URL, logger) + + actual, err := api.TeamInfo("acid", token) + if err != nil && err.Error() != tc.err.Error() { + t.Errorf("Expected error: %v, got: %v", tc.err, err) + return + } + + if !reflect.DeepEqual(actual, tc.out) { + t.Errorf("Expected %#v, got: %#v", tc.out, actual) + } + }() + } +} + +func TestRequest(t *testing.T) { + for _, tc := range requestsURLtc { + api := NewTeamsAPI(tc.url, logger) + resp, err := api.TeamInfo("acid", token) + if resp != nil { + t.Errorf("Response expected to be nil") + continue + } + + if err.Error() != tc.err.Error() { + t.Errorf("Expected error: %v, got: %v", tc.err, err) + } + } +}