PR: Feature Request - New Resource Notification Policy and Label selector endpoint #308

samarsinghal · 2020-10-26T21:14:50Z

This PR include the below changes:-

Message policy to send an email notification to org owners. The org owners are configured as "PrimaryOwner" and "SecondaryOwner" labels on each organization.

…ferings

pacphi

Nice work here. I think this is the realization of a long-standing feature request, as documented here: #10. But it's not an action-based policy rather only to be used for reporting and notification.

pacphi · 2020-10-27T02:24:05Z

README.md

+    {
+    "message-policies": [
+        {
+            "id": "b56fdcce40fd0dbe1c74791343850f6881595b26",


Is this attribute id truly needed? Probably ok to remove.

pacphi · 2020-10-27T02:25:15Z

README.md

@@ -236,6 +236,17 @@ As mentioned previously the policy file must adhere to a naming convention
 See additional property requirements in Query policies and the aforementioned sample Github repository.


+#### Message Policies


Is Message Policy really the right name for this?

What content are we attaching to send to the org owners?

I think I see what you're trying to do. You're trying to add a label filter, right? As in you're trying to implement this feature: #10? Maybe this should be renamed to Label Policy?

Changed the name to ResourceNotificationPolicy

pacphi · 2020-10-27T02:27:19Z

src/main/java/io/pivotal/cfapp/controller/DbmsOnlyPoliciesController.java

@@ -43,6 +43,12 @@ public DbmsOnlyPoliciesController(
                .map(ResponseEntity::ok);
    }

+    @DeleteMapping("/policies/message/{id}")


Not convinced on the policy type = message for this policy. We need to find a better type name.

Likely message should be renamed to label.

Changed the name to ResourceNotificationPolicy

pacphi · 2020-10-27T02:35:36Z

src/main/java/io/pivotal/cfapp/service/ResourceMetadataService.java

+                    .scheme("https")
+                    .host(settings.getApiHost())
+                    .path("/v3/{type}")
+                    .queryParam("label_selector", labelSelector)


This is the crux of what you're wanting to return to the interested party (org owner)? It's the resource(s) that match(es) the label selector?

pacphi · 2020-10-27T02:41:03Z

src/main/java/io/pivotal/cfapp/domain/PoliciesValidator.java

@@ -153,6 +153,22 @@ public boolean validate(HygienePolicy policy) {
        return valid;
    }

+    public boolean validate(MessagePolicy policy) {


Need a new unit test here: https://github.com/pacphi/cf-butler/blob/master/src/test/java/io/pivotal/cfapp/domain/PoliciesValidatorTest.java

pacphi · 2020-10-27T02:42:28Z

src/main/java/io/pivotal/cfapp/repository/R2dbcPoliciesRepository.java

@@ -74,6 +76,15 @@ public R2dbcPoliciesRepository(
                .then();
    }

+    public Mono<Void> deleteMessagePolicyById(String id) {


I know we don't yet have unit tests for each policy type, but we really should add one. Happy to collaborate on authoring one with you. It would go in here: https://github.com/pacphi/cf-butler/tree/master/src/test/java/io/pivotal/cfapp/repository.

pacphi

Just a few questions and some wording updates in documentation. We're almost there!

pacphi · 2020-11-03T01:24:31Z

README.md

@@ -236,6 +236,17 @@ As mentioned previously the policy file must adhere to a naming convention
 See additional property requirements in Query policies and the aforementioned sample Github repository.


+#### Resource Notification Policies
+
+Resource Notification policies are useful when you wants to send notification for a resource(Org, Apps, Services) to the emailid's configured on the resources as labels. 


when you want to generate a report containing resources of a particular type and send that report to email recipients identified by one or more label values ascribed to the those resources.

For example, you might notify primary and/or secondary owners of a collection of a resources.

pacphi · 2020-11-03T01:25:56Z

README.md

+                            "PrimaryOwner",
+                            "SecondaryOwner"
+                          ],
+                "domain": "pivotal.io"


Please let's be explicit... update domain be email_domain.

Done changed domain attribute to email-domain

pacphi · 2020-11-03T01:26:44Z

README.md

+                "body": "Please take a moment to review the platform updates and share it with your Org users"
+            },
+            "resource-email-metadata": {
+                "resource": "organizations",


I like that you limited metadata definition to one resource type.

Are you sure that the resource type is plural form? Or is it really singular form? E.g., instead of organizations would it be organization?

I see it's actually what's allowed in ResourceType enum. You're good in this example.

…omain

samar singhal and others added 18 commits May 6, 2020 22:19

Added latest buildpack details

6a24811

Added latest buildpack details to AI snapshot report

cec3711

Added legacy service policy

cb573a1

Changes services to service-offerings

f979fe9

removed logs

e80de33

Refactor AppDetailTask events

711e20d

Implemented review comments and updated serviceOffering to service-of…

cce0f82

…ferings

Fixed legacy service offering email

9dede14

Merge branch 'master' of https://github.com/pacphi/cf-butler

53a8924

git sync

026ce14

git sync

6569fae

Merge remote-tracking branch 'upstream/master'

8a0cc20

Merge remote-tracking branch 'upstream/master' into master

90df6bf

Added support for label-selector

0218094

resolved merge issues

c070c54

Added Message Policy

eb9d0e8

Merge remote-tracking branch 'upstream/master' into master

914c237

Updated readme for Message policy

7ab6d61

pacphi reviewed Oct 27, 2020

View reviewed changes

Updated message policy to support all resources

af11baa

pacphi reviewed Nov 3, 2020

View reviewed changes

Changed resource notification policy domain attribute name to email-d…

c63c68d

…omain

pacphi changed the title ~~Pull request for message policy and label selector endpoint~~ PR: Feature Request - New Resource Notification Policy and Label selector endpoint Nov 3, 2020

pacphi merged commit 845dded into cf-toolsuite:master Nov 3, 2020

		@@ -236,6 +236,17 @@ As mentioned previously the policy file must adhere to a naming convention
		See additional property requirements in Query policies and the aforementioned sample Github repository.


		#### Message Policies

PR: Feature Request - New Resource Notification Policy and Label selector endpoint #308

PR: Feature Request - New Resource Notification Policy and Label selector endpoint #308

Uh oh!

Conversation

samarsinghal commented Oct 26, 2020

Uh oh!

pacphi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pacphi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!