Skip to content
This repository has been archived by the owner on Oct 11, 2019. It is now read-only.

VYGR-391: Add opsgenie integrations to notifications #120

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
005de8c
VYGR-391: Create OpsGenie integrations secret during sync
fraser-atlassian Feb 5, 2019
f37c6fa
VYGR-391: Use SecretTypeOpaque for opsgenie secret
fraser-atlassian Feb 13, 2019
fcb59fa
VYGR-391: Add opsgenie integrations into notifications
fraser-atlassian Feb 15, 2019
45259db
VYGR-391: OpsGenie -> Opsgenie
fraser-atlassian Feb 15, 2019
dd63115
VYGR-391: Set Opsgenie metadata in serviceDataToService
fraser-atlassian Feb 15, 2019
46e0cfb
VYGR-391: Use all service attributes in service object
fraser-atlassian Feb 15, 2019
b7e6865
VYGR-391: Use constants for default pagerduty notifications
fraser-atlassian Feb 15, 2019
cd3ba39
VYGR-391: Override pagerduty defaults instead of new object
fraser-atlassian Feb 15, 2019
9022baf
VYGR-391: Pull build opsgenie notifcations into separate function
fraser-atlassian Feb 15, 2019
0d61496
Merge branch 'master' into fcobb/VYGR-391-save-opsgenie-secret-during…
fraser-atlassian Feb 15, 2019
4844d76
VYGR-391: Comment on optional opsgenie metadata
fraser-atlassian Feb 18, 2019
533eb33
VYGR-391: Log when unable to get Opsgenie attribute in SC store
fraser-atlassian Feb 18, 2019
ce4b544
VYGR-391: Error when Opsgenie filtering encounters unexpected envType
fraser-atlassian Feb 18, 2019
9368162
VYGR-391: Return error when we failed to get Opsgenie attr
fraser-atlassian Feb 18, 2019
d13b742
VYGR-391: Remove struct method usage for serviceDataToService
fraser-atlassian Feb 18, 2019
c811811
VYGR-391: Reorder url errors to return error earlier
fraser-atlassian Feb 18, 2019
0271bef
VYGR-391: Return nil for error cases in GetServiceAttributes
fraser-atlassian Feb 18, 2019
41066cb
VYGR-391: Add unit tests for opsgenie buildNotifications
fraser-atlassian Feb 18, 2019
964933c
VYGR-391: Update sync bazel
fraser-atlassian Feb 18, 2019
e3e587b
VYGR-391: Add test for Opsgenie build notifications per env
fraser-atlassian Feb 19, 2019
434198e
Merge branch 'master' into fcobb/VYGR-391-save-opsgenie-secret-during…
fraser-atlassian Feb 19, 2019
f229991
VYGR-391: Add Opsgenie integrations into entangler test
fraser-atlassian Feb 21, 2019
a5ac769
VYGR-391: Reorder entangler_test imports
fraser-atlassian Feb 21, 2019
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
1 change: 1 addition & 0 deletions cmd/synchronization/app/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ go_library(
"//pkg/composition/client:go_default_library",
"//pkg/k8s:go_default_library",
"//pkg/k8s/updater:go_default_library",
"//pkg/opsgenie:go_default_library",
"//pkg/options:go_default_library",
"//pkg/releases:go_default_library",
"//pkg/releases/deployinator/client:go_default_library",
Expand Down
6 changes: 6 additions & 0 deletions cmd/synchronization/app/controller_constructor.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
comp_v1_client "github.com/atlassian/voyager/pkg/composition/client"
"github.com/atlassian/voyager/pkg/k8s"
"github.com/atlassian/voyager/pkg/k8s/updater"
"github.com/atlassian/voyager/pkg/opsgenie"
"github.com/atlassian/voyager/pkg/releases"
"github.com/atlassian/voyager/pkg/releases/deployinator/client"
"github.com/atlassian/voyager/pkg/servicecentral"
Expand Down Expand Up @@ -79,6 +80,10 @@ func (cc *ControllerConstructor) New(config *ctrl.Config, cctx *ctrl.Context) (*
scHTTPClient := util.HTTPClient()
scClient := servicecentral.NewServiceCentralClient(config.Logger, scHTTPClient, opts.ASAPClientConfig, opts.Providers.ServiceCentralURL)

// create a client for talking to Opsgenie Integration Manager
ogHTTPClient := util.HTTPClient()
fcobb marked this conversation as resolved.
Show resolved Hide resolved
ogClient := opsgenie.New(config.Logger, ogHTTPClient, opts.ASAPClientConfig, opts.Providers.OpsgenieIntegrationsManagerURL)

scErrorCounter := prometheus.NewCounter(
prometheus.CounterOpts{
Namespace: config.AppName,
Expand Down Expand Up @@ -133,6 +138,7 @@ func (cc *ControllerConstructor) New(config *ctrl.Config, cctx *ctrl.Context) (*
ServiceCentral: servicecentral.NewStore(config.Logger, scClient),
ReleaseManagement: releases.NewReleaseManagement(deployinatorHTTPClient, config.Logger),
ClusterLocation: opts.Location.ClusterLocation(),
Opsgenie: ogClient,

ConfigMapUpdater: configMapObjectUpdater,
RoleBindingUpdater: roleBindingObjectUpdater,
Expand Down
29 changes: 23 additions & 6 deletions cmd/synchronization/app/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,29 +21,42 @@ type Options struct {
}

type Providers struct {
ServiceCentralURL *url.URL // we use custom json marshalling to read it
DeployinatorURL *url.URL
ServiceCentralURL *url.URL // we use custom json marshalling to read it
DeployinatorURL *url.URL
OpsgenieIntegrationsManagerURL *url.URL
}

// UnmarshalJSON unmarshals our untyped config file into a typed struct including URLs
func (p *Providers) UnmarshalJSON(data []byte) error {
var rawProviders struct {
ServiceCentral string `json:"serviceCentral"`
Deployinator string `json:"deployinator"`
ServiceCentral string `json:"serviceCentral"`
Deployinator string `json:"deployinator"`
OpsgenieIntegrationsManager string `json:"opsgenieIntegrationsManager"`
fcobb marked this conversation as resolved.
Show resolved Hide resolved
}

if err := json.Unmarshal(data, &rawProviders); err != nil {
return err
}

scURL, err := url.Parse(rawProviders.ServiceCentral)
p.ServiceCentralURL = scURL
if err != nil {
return errors.Wrap(err, "unable to parse Service Central URL")
}
p.ServiceCentralURL = scURL

depURL, err := url.Parse(rawProviders.Deployinator)
if err != nil {
fcobb marked this conversation as resolved.
Show resolved Hide resolved
return errors.Wrap(err, "unable to parse Deployinator URL")
}
Copy link
Author

@fcobb fcobb Feb 18, 2019

Choose a reason for hiding this comment

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

I am not sure why this always returned an error and I cannot identify where this function is called.
I think its called in readAndValidateOptions when yaml.UnmarshalStrict is called, but if thats the case then I'm not sure why we always fail.
https://github.com/atlassian/voyager/blob/master/cmd/synchronization/app/options.go#L67

@ash2k Can you help me understand why we returned an error before?

Copy link
Contributor

Choose a reason for hiding this comment

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

seems like a bug, not sure how did it work

Copy link
Contributor

Choose a reason for hiding this comment

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

p.DeployinatorURL = depURL
return errors.Wrap(err, "unable to parse Deployinator URL")

ogUrl, err := url.Parse(rawProviders.OpsgenieIntegrationsManager)
if err != nil {
return errors.Wrap(err, "unable to parse Opsgenie Integrations Manager URL")
}
p.OpsgenieIntegrationsManagerURL = ogUrl

return nil
}

func (o *Options) DefaultAndValidate() []error {
Expand All @@ -54,6 +67,10 @@ func (o *Options) DefaultAndValidate() []error {
allErrors = append(allErrors, errors.New("providers.serviceCentral must be a valid URL"))
}

if o.Providers.OpsgenieIntegrationsManagerURL == nil {
allErrors = append(allErrors, errors.New("providers.OpsgenieIntegrationsManagerURL must be a valid URL"))
}

return allErrors
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/creator/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ func (ss *ServiceSpec) EmailAddress() string {
// +k8s:deepcopy-gen=true
type ServiceMetadata struct {
PagerDuty *PagerDutyMetadata `json:"pagerDuty,omitempty"`
Opsgenie *OpsgenieMetadata `json:"opsgenie,omitempty"`
Bamboo *BambooMetadata `json:"bamboo,omitempty"`
}

Expand Down Expand Up @@ -100,6 +101,10 @@ type PagerDutyIntegrationMetadata struct {
IntegrationKey string `json:"integrationKey,omitempty"`
}

type OpsgenieMetadata struct {
Team string `json:"team,omitempty"`
}

// +k8s:deepcopy-gen=true
type Compliance struct {
PRGBControl *bool `json:"prgbControl,omitempty"`
Expand Down
5 changes: 4 additions & 1 deletion pkg/apis/orchestration/meta/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,8 @@ go_library(
srcs = ["types.go"],
importpath = "github.com/atlassian/voyager/pkg/apis/orchestration/meta",
visibility = ["//visibility:public"],
deps = ["//:go_default_library"],
deps = [
"//:go_default_library",
"//pkg/opsgenie:go_default_library",
],
)
12 changes: 8 additions & 4 deletions pkg/apis/orchestration/meta/types.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package meta

import "github.com/atlassian/voyager"
import (
"github.com/atlassian/voyager"
"github.com/atlassian/voyager/pkg/opsgenie"
)

const (
ConfigMapConfigKey = "config"
Expand All @@ -27,9 +30,10 @@ type ServiceProperties struct {

// Notification is used in the ServiceProperties.
type Notifications struct {
Email string `json:"email"`
LowPriorityPagerdutyEndpoint PagerDuty `json:"lowPriority"`
PagerdutyEndpoint PagerDuty `json:"main"`
Email string `json:"email"`
LowPriorityPagerdutyEndpoint PagerDuty `json:"lowPriority"`
PagerdutyEndpoint PagerDuty `json:"main"`
OpsgenieIntegrations []opsgenie.Integration `json:"opsgenieIntegrations"`
}

// PagerDuty is used in the ServiceProperties.
Expand Down
2 changes: 1 addition & 1 deletion pkg/opsgenie/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func New(logger *zap.Logger, httpClient *http.Client, asap pkiutil.ASAP, baseURL
}
}

// Gets OpsGenie integrations
// Gets Opsgenie integrations
// return codes:
// - 400: Bad request to Opsgenie
// - 401: Unauthorized
Expand Down
10 changes: 5 additions & 5 deletions pkg/opsgenie/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestGetIntegrations(t *testing.T) {
defer ogIntManagerMockServer.Close()

// when
ogIntManagerClient := mockOpsGenieIntegrationManagerClient(t, ogIntManagerMockServer.URL, pkitest.MockASAPClientConfig(t))
ogIntManagerClient := mockOpsgenieIntegrationManagerClient(t, ogIntManagerMockServer.URL, pkitest.MockASAPClientConfig(t))
_, retriable, err := ogIntManagerClient.GetOrCreateIntegrations(context.Background(), teamName)

// then
Expand All @@ -58,7 +58,7 @@ func TestGetIntegrationsTeamNotFound(t *testing.T) {
defer ogIntManagerMockServer.Close()

// when
ogIntManagerClient := mockOpsGenieIntegrationManagerClient(t, ogIntManagerMockServer.URL, pkitest.MockASAPClientConfig(t))
ogIntManagerClient := mockOpsgenieIntegrationManagerClient(t, ogIntManagerMockServer.URL, pkitest.MockASAPClientConfig(t))
_, retriable, err := ogIntManagerClient.GetOrCreateIntegrations(context.Background(), teamName)

// then
Expand All @@ -83,7 +83,7 @@ func TestGetIntegrationsRateLimited(t *testing.T) {
defer ogIntManagerMockServer.Close()

// when
ogIntManagerClient := mockOpsGenieIntegrationManagerClient(t, ogIntManagerMockServer.URL, pkitest.MockASAPClientConfig(t))
ogIntManagerClient := mockOpsgenieIntegrationManagerClient(t, ogIntManagerMockServer.URL, pkitest.MockASAPClientConfig(t))
_, retriable, err := ogIntManagerClient.GetOrCreateIntegrations(context.Background(), teamName)

// then
Expand All @@ -108,7 +108,7 @@ func TestGetIntegrationsInternalServerError(t *testing.T) {
defer ogIntManagerMockServer.Close()

// when
ogIntManagerClient := mockOpsGenieIntegrationManagerClient(t, ogIntManagerMockServer.URL, pkitest.MockASAPClientConfig(t))
ogIntManagerClient := mockOpsgenieIntegrationManagerClient(t, ogIntManagerMockServer.URL, pkitest.MockASAPClientConfig(t))
_, retriable, err := ogIntManagerClient.GetOrCreateIntegrations(context.Background(), teamName)

// then
Expand All @@ -117,7 +117,7 @@ func TestGetIntegrationsInternalServerError(t *testing.T) {
require.True(t, retriable)
}

func mockOpsGenieIntegrationManagerClient(t *testing.T, serverMockAddress string, asap pkiutil.ASAP) *Client {
func mockOpsgenieIntegrationManagerClient(t *testing.T, serverMockAddress string, asap pkiutil.ASAP) *Client {
opsgenieIntegrationManagerURL, err := url.Parse(serverMockAddress)
require.NoError(t, err)
httpClient := util.HTTPClient()
Expand Down
4 changes: 2 additions & 2 deletions pkg/opsgenie/it/client_manual_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
)

const (
opsGenieIntManURL = "https://micros.prod.atl-paas.net"
opsgenieIntManURL = "https://micros.prod.atl-paas.net"
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use real data in tests please. Use fake data =)

Copy link
Author

Choose a reason for hiding this comment

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

This is a manual integration test, I don't think fake data is applicable.

)

// NOTE: THIS WILL CREATE INTEGRATIONS IF NONE EXIST
Expand All @@ -32,7 +32,7 @@ func TestGetIntegrations(t *testing.T) {
require.NoError(t, asapErr)

client := util.HTTPClient()
c := opsgenie.New(testLogger, client, asapConfig, parseURL(t, opsGenieIntManURL))
c := opsgenie.New(testLogger, client, asapConfig, parseURL(t, opsgenieIntManURL))

// Get Service Attributes
resp, _, err := c.GetOrCreateIntegrations(ctx, "Platform SRE")
Expand Down
27 changes: 18 additions & 9 deletions pkg/opsgenie/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,22 @@ type IntegrationsResponse struct {
}

type Integration struct {
ID string `json:"id"`
Name string `json:"name"`
Type string `json:"type"`
TeamID string `json:"teamId"`
TeamName string `json:"teamName"`
Priority string `json:"priority"`
APIKey string `json:"apiKey"`
Endpoint string `json:"endpoint"`
EnvType string `json:"envType"`
ID string `json:"id"`
Name string `json:"name"`
Type string `json:"type"`
TeamID string `json:"teamId"`
TeamName string `json:"teamName"`
Priority string `json:"priority"`
APIKey string `json:"apiKey"`
Endpoint string `json:"endpoint"`
EnvType EnvType `json:"envType"`
}

type EnvType string

const (
EnvTypeDev EnvType = "dev"
EnvTypeStaging EnvType = "staging"
EnvTypeProd EnvType = "prod"
EnvTypeGlobal EnvType = "null" // Intentionally a string called "null" as this is the expected result from opsgenie int manager
Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not global?

Copy link
Author

@fcobb fcobb Feb 18, 2019

Choose a reason for hiding this comment

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

I am not sure why its not called "global". @jokeyrhyme Do you know why?

Choose a reason for hiding this comment

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

@fcobb Well, it's not that things are global, it's just that they aren't explicitly environment-specific
Maybe this should actually mean Global, and maybe we should actually emit "global" instead of "null"
When we put this together, the most important thing was making sure our CloudWatch alarms (which are environment-specific) were covered, everything else was secondary
I'm wondering whether we should call this Null or something so that we can meaningfully transition to Global in future?

Copy link
Author

Choose a reason for hiding this comment

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

@jokeyrhyme I went for Global as if its not environment specific then its essentially global.
As you own this API I will defer these decisions to you and your team. My preference is to return a specific value for these non-environment specific values rather than null.

)
32 changes: 13 additions & 19 deletions pkg/servicecentral/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,8 @@ func (c *Client) GetService(ctx context.Context, user auth.OptionalUser, service
if err != nil {
return nil, errors.Wrap(err, "failed to get attributes for service")
}
ogTeamAttr, found, err := findOpsGenieTeamServiceAttribute(resp)
if err != nil {
return nil, errors.Wrap(err, "failed to get OpsGenie attributes for service")
}

if found {
service.Attributes = append(service.Attributes, ogTeamAttr)
}
Copy link
Author

Choose a reason for hiding this comment

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

Previously, we filtered the service attributes before adding them to the service object. We now do that filtering later on.

service.Attributes = resp

return service, nil
}
Expand Down Expand Up @@ -344,7 +338,7 @@ func (c *Client) DeleteService(ctx context.Context, user auth.User, serviceUUID
}

// GetServiceAttributes queries service central for the attributes of a given service. Can return an empty array if no attributes were found
func (c *Client) GetServiceAttributes(ctx context.Context, user auth.OptionalUser, serviceUUID string) ([]ServiceAttributeResponse, error) {
func (c *Client) GetServiceAttributes(ctx context.Context, user auth.OptionalUser, serviceUUID string) ([]ServiceAttribute, error) {
req, err := c.rm.NewRequest(
pkiutil.AuthenticateWithASAP(c.asap, asapAudience, user.NameOrElse(noUser)),
restclient.Method(http.MethodGet),
Expand All @@ -371,14 +365,13 @@ func (c *Client) GetServiceAttributes(ctx context.Context, user auth.OptionalUse
message := fmt.Sprintf("failed to get attributes for service %q. Response: %s", serviceUUID, respBody)
return nil, clientError(response.StatusCode, message)
}

var parsedBody []ServiceAttributeResponse
err = json.Unmarshal(respBody, &parsedBody)
var svcAttrResp []ServiceAttribute
err = json.Unmarshal(respBody, &svcAttrResp)
if err != nil {
return nil, errors.Wrap(err, "failed to unmarshal response body")
}

return parsedBody, nil
return svcAttrResp, nil
}

func clientError(statusCode int, message string) error {
Expand Down Expand Up @@ -445,27 +438,28 @@ func convertV2ServiceToV1(v2Service V2Service) ServiceDataRead {
return service
}

func findOpsGenieTeamServiceAttribute(attributes []ServiceAttributeResponse) (ServiceAttribute, bool /*found*/, error) {
const opsGenieSchemaName = "opsgenie"
// findOpsgenieAttribute searches a given list of ServiceAttributes for a single OpsgenieAttribute
func findOpsgenieAttribute(attributes []ServiceAttribute) (OpsgenieAttribute, bool /*found*/, error) {
const opsgenieSchemaName = "opsgenie"
count := 0
found := false
ogTeamAttr := ServiceAttribute{}
ogTeamAttr := OpsgenieAttribute{}
for _, attr := range attributes {
if attr.Schema.Name != opsGenieSchemaName {
if attr.Schema.Name != opsgenieSchemaName {
continue
}

team, ok := attr.Value["team"]
if !ok {
return ogTeamAttr, found, errors.Errorf("expected to find team name within schema of name %q", opsGenieSchemaName)
return ogTeamAttr, found, errors.Errorf("expected to find team name within schema of name %q", opsgenieSchemaName)
}

ogTeamAttr = ServiceAttribute{Team: team}
ogTeamAttr = OpsgenieAttribute{Team: team}
found = true
count++
}
if count > 1 {
return ogTeamAttr, found, errors.New("found more than one OpsGenie service attribute")
return ogTeamAttr, found, errors.New("found more than one Opsgenie service attribute")
}
return ogTeamAttr, found, nil
}
Loading