Skip to content
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

EscalationRule struct should accept a slice of APIReference rather then APIObject for Targets #316

Closed
gerardocorea opened this issue Mar 22, 2021 · 7 comments · Fixed by #332
Milestone

Comments

@gerardocorea
Copy link
Contributor

gerardocorea commented Mar 22, 2021

While attempting to configure an escalation policy I continued to receive the following error below.

HTTP response failed with status code 400, message: Invalid Input Provided (code: 2001)

upon further research in the api documentation it shows that the fields Targets expects an array of targets with an id and type being required. type should be set to user_reference

https://developer.pagerduty.com/api-reference/reference/REST/openapiv3.json/paths/~1escalation_policies/post

The GetUser method returns a User struct with an APIObject struct. The type field not populated in the GetUser response which results in this field being omitted in the post call for an escalation policy.

Example post call below. As you can see a value of APIObject in the targets array is missing the type field.

{
   "escalation_policy":{
      "name":"Test",
      "escalation_rules":[
         {
            "escalation_delay_in_minutes":30,
            "targets":[
               {
                  "id":"OMMITTED",
                  "self":"https://api.pagerduty.com/users/OMMITED",
                  "html_url":"https://ukg-dev.pagerduty.com/users/OMMITED"
               }
            ]
         }
      ],
      "teams":[
         {
            "id":"OMITTED",
            "type":"team_reference"
         }
      ]
   }
}

Manually assigning the Type field results in the successful creation of the escalation policy

defaultUser, err := client.GetUser(testUserID, pagerduty.GetUserOptions{})
defaultTargets := []pagerduty.APIObject{}
defaultTargets = append(defaultTargets, pagerduty.APIObject{ID: defaultUser.ID, Type: "user_reference"})
@theckman
Copy link
Collaborator

theckman commented Mar 23, 2021

@gerardocorea your example snippet seems to omit some things. Which method(s) did you call with which input? I'm not fully grokking what's amiss here.

I'm not sure the title is correct at a minimum, as the PagerDuty API reference you linked shows that targets entities have all of the same fields as https://pkg.go.dev/github.com/PagerDuty/go-pagerduty#APIObject and not just the subset within https://pkg.go.dev/github.com/PagerDuty/go-pagerduty#APIReference

@gerardocorea
Copy link
Contributor Author

@gerardocorea your example snippet seems to omit some things. Which method(s) did you call with which input? I'm not fully grokking what's amiss here.

I'm not sure the title is correct at a minimum, as the PagerDuty API reference you linked shows that targets entities have all of the same fields as https://pkg.go.dev/github.com/PagerDuty/go-pagerduty#APIObject and not just the subset within https://pkg.go.dev/github.com/PagerDuty/go-pagerduty#APIReference

@theckman the issue I see is that GetUser method returns a User object with an APIObject in which the type field is not populated. This causes the call to create an escalation policy to fail since the type is a required field. Your right that the title might not be right since an APIObject has both the ID and Type fields so I can adjust that as needed.

When trying to create the escalationPolicy I did the following below.

	defaultUser, err := client.GetUser(testUserID, pagerduty.GetUserOptions{})
	defaultTargets := []pagerduty.APIObject{}
	defaultTargets = append(defaultTargets, defaultUser.APIObject)


	escalationPolicy := pagerduty.EscalationPolicy{
		Name: escalationPolicyName,
		Teams: []pagerduty.APIReference{
			{
				ID: "PBRIIG8",
				Type: "team_reference",
			},
		},
		EscalationRules: []pagerduty.EscalationRule{
			escalationRule,
		},
	}

Which resulted in the error below

HTTP response failed with status code 400, message: Invalid Input Provided (code: 2001)

unless I appended to the defaultTargets slice this way

defaultTargets = append(defaultTargets, pagerduty.APIObject{ID: defaultUser.ID, Type: "user_reference"})

Im wondering now if instead its simply an unmarshal issue or if the type is not being sent in the response correctly which wouldnt be something we would likely address here. Hopefully that clears it up a bit.

@theckman
Copy link
Collaborator

theckman commented Mar 23, 2021

@gerardocorea I'm pretty sure that's what you have to do, unfortunately. GetUser from their REST API doesn't return a user_reference it returns a full user. So you need to update it to indicate it's only a reference for the API you are then calling to accept it.

Based on their API documentation, and how we've defined the type, it should be populated with the value user.

@theckman
Copy link
Collaborator

theckman commented Mar 23, 2021

Ah I see now, it's probably a similar issue as #218. There are two Type fields, one on the User object and one on the APIObject embedded within. This is a side effect of embedding trying to be used as inheritance, and we'll need to fix it as a breaking change.

@theckman
Copy link
Collaborator

To be clear, once fixed this will avoid the field being unset. You will still probably have to update it to be a user_reference so the API accepts it.

@gerardocorea
Copy link
Contributor Author

To be clear, once fixed this will avoid the field being unset. You will still probably have to update it to be a user_reference so the API accepts it.

Noted. If you think its fine to close this issue then we can.

@theckman
Copy link
Collaborator

theckman commented Apr 2, 2021

I'm going to keep it open until we merge a PR to address it. I should start to work on v1.5.0 once all of the v1.4.0 open items are addressed. Ideally within the coming weeks...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants