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

Add support for Webhooks #169

Merged
merged 2 commits into from Sep 2, 2019

Conversation

@codenio
Copy link
Contributor

@codenio codenio commented Aug 30, 2019

ISSUE TYPE
  • Feature Pull Request
SUMMARY

Modify return type as pointer:

This commit,

  • modifies return type of GetLastSeenSlackMessage to pointers.

Add support for Webhooks:

This Commit,

  • Adds support for event notifications through Wehbook
  • updates configs in all yaml files
  • updated helm charts
  • adds unit tests for webhook notifier
  • adds integration test webhook notifier
  • adds test/webhook package to enable integration testing.

Preview:

{
  "meta": {
    "kind": "Pod",
    "name": "command-app-1",
    "namespace": "default",
    "cluster": "demo"
  },
  "status": {
    "type": "create",
    "level": "info",
    "reason": "",
    "messages": [
      "Resource created\n"
    ]
  },
  "summary": "Pod `command-app-1` in of cluster `demo`, namespace `default` has been created:\n```Resource created\nRecommendations:\n- :latest tag used in image 'debian' of Container 'command-demo-container' should be avoided.\n```",
  "timestamp": "2019-08-30T22:29:04+05:30",
  "recommendations": [
    ":latest tag used in image 'debian' of Container 'command-demo-container' should be avoided.\n"
  ]
}
@codenio codenio force-pushed the feature/webhook-support branch from 6786084 to 3b41fe6 Aug 30, 2019
@sanketsudake sanketsudake requested a review from PrasadG193 Aug 31, 2019
@sanketsudake
Copy link
Member

@sanketsudake sanketsudake commented Aug 31, 2019

@aananthraj
Lets add unit tests and integration tests as well. We can mock webhook URL and notifier would call that webhook URL with payload. We can verify cross check payload received on Webhook and sent from notifier.

You can also refer integration tests recently added by @PrasadG193

@sanketsudake sanketsudake self-requested a review Aug 31, 2019
@codenio codenio force-pushed the feature/webhook-support branch from 3b41fe6 to 65eca93 Sep 1, 2019
@codenio
Copy link
Contributor Author

@codenio codenio commented Sep 1, 2019

@sanketsudake, @PrasadG193 made those requested changes.

test/e2e/notifier/create/create.go Show resolved Hide resolved
test/e2e/env/env.go Outdated Show resolved Hide resolved
@PrasadG193
Copy link
Member

@PrasadG193 PrasadG193 commented Sep 1, 2019

Minor comment, can we rename test/webhooktest package to test/webhook to maintain consistency in the naming format of all test/* packages

@codenio
Copy link
Contributor Author

@codenio codenio commented Sep 1, 2019

Sure, lets maintain the same.
Let know in case of further changes (if any)

codenio added 2 commits Sep 1, 2019
This commit,
- modifies return type of  GetLastSeenSlackMessage to pointers.
This commit,
- Adds support for event notifications through `Wehbook` 
- updates configs in all yaml files
- updated helm charts
- adds unit tests for webhook notifier
- adds integration test webhook notifier
- adds test/webhook package to enable integration testing.
@codenio codenio force-pushed the feature/webhook-support branch from 65eca93 to 3b0a672 Sep 1, 2019
@codenio
Copy link
Contributor Author

@codenio codenio commented Sep 1, 2019

made those request changes.
It is ready now, let's get it merged.

@PrasadG193
Copy link
Member

@PrasadG193 PrasadG193 commented Sep 2, 2019

Looks good for now, we might need to add support for the self-signed certificates in future

@PrasadG193 PrasadG193 merged commit 9800db7 into infracloudio:develop Sep 2, 2019
1 check passed
@codenio codenio deleted the feature/webhook-support branch Sep 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants