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 webhook domain exclusion config for notification channels #172

Merged
merged 5 commits into from
Jul 20, 2023

Conversation

pavan-traceable
Copy link
Contributor

Description

Add domain exclusion config for notification channel config service. This config will fail the creation/update of notification channels if it contains exclusion domains.

@pavan-traceable pavan-traceable requested a review from a team as a code owner July 11, 2023 11:52
@github-actions
Copy link

github-actions bot commented Jul 11, 2023

Test Results

115 tests  +2   115 ✔️ +2   1m 32s ⏱️ +18s
  29 suites +1       0 💤 ±0 
  29 files   +1       0 ±0 

Results for commit c560380. ± Comparison against base commit 93f14a5.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Merging #172 (c560380) into main (93f14a5) will increase coverage by 0.13%.
The diff coverage is 87.75%.

@@             Coverage Diff              @@
##               main     #172      +/-   ##
============================================
+ Coverage     79.76%   79.90%   +0.13%     
+ Complexity      465      389      -76     
============================================
  Files            54       54              
  Lines          2333     2379      +46     
  Branches         92      103      +11     
============================================
+ Hits           1861     1901      +40     
- Misses          419      420       +1     
- Partials         53       58       +5     
Flag Coverage Δ
integration 79.90% <87.75%> (+0.13%) ⬆️
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../service/NotificationChannelConfigServiceImpl.java 80.23% <66.66%> (-1.70%) ⬇️
...ificationChannelConfigServiceRequestValidator.java 70.32% <90.69%> (+18.24%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

validateRequestContextOrThrow(requestContext);
validateNotificationChannelMutableData(request.getNotificationChannelMutableData());
validateWebhookConfigExclusionDomains(

Choose a reason for hiding this comment

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

Please add unit tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. Added validation for https support

avinashkolluru
avinashkolluru previously approved these changes Jul 19, 2023
@avinashkolluru avinashkolluru dismissed their stale review July 19, 2023 14:59

vulnerability needs to be fixed

@avinashkolluru
Copy link

guava-31.1-android.jar (pkg:maven/com.google.guava/guava@31.1-android, cpe:2.3:a:google:guava:31.1:::::::*) : CVE-2023-2976

@pavan-traceable this needs to be fixed. the android version of guava dep seems to be coming somehow. Please check.

@pavan-traceable pavan-traceable merged commit e192b8c into main Jul 20, 2023
@pavan-traceable pavan-traceable deleted the domain-exclusion-notification-channel-config branch July 20, 2023 11:22
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 this pull request may close these issues.

2 participants