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.

Sign up for GitHub

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

[PoC] Security Group Filtering #53

Closed
wants to merge 18 commits into from
Closed

[PoC] Security Group Filtering #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
5cae65b
adding event switch
Keyli0Iliev Nov 22, 2022
68a8ef3
Update desiredLRP handler
Keyli0Iliev Nov 22, 2022
3ae62a2
Update 0 to true
Keyli0Iliev Nov 30, 2022
c102bc3
set to expect not zero rules
Keyli0Iliev Dec 1, 2022
9ca5680
add case switch
Keyli0Iliev Dec 14, 2022
6effbbc
Merge pull request #1 from Keyli0Iliev/patch-1
Keyli0Iliev Dec 14, 2022
59cd065
Update desired_lrp_handlers.go
Keyli0Iliev Dec 14, 2022
3190a5b
adding skip Egress Rules to DesiredLRPFilter
Keyli0Iliev Dec 21, 2022
b0a4a03
Update desired_lrp_handlers.go
Keyli0Iliev Dec 21, 2022
bf7749c
Regenerating protobuff files
Keyli0Iliev Dec 21, 2022
38a37d4
Update desired_lrp_requests.proto
Keyli0Iliev Dec 21, 2022
82124a6
Adding SkipEgressRules to DesiredLRP.go
Keyli0Iliev Dec 21, 2022
229aec2
Update events_handlers_r0.go
Keyli0Iliev Dec 21, 2022
f8c6902
Switch SkipEgressRules from req to filter
Keyli0Iliev Dec 23, 2022
48ffe35
Update desired_lrp_handlers.go
Keyli0Iliev Jan 11, 2023
f6d2ebd
adding console logs
Keyli0Iliev Jan 11, 2023
7a608b7
Adding fmt import
Keyli0Iliev Jan 11, 2023
6de772c
Remove Test logging
Keyli0Iliev Jan 13, 2023
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
5 changes: 3 additions & 2 deletions client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,9 @@ func (c *client) RemoveEvacuatingActualLRP(logger lager.Logger, key *models.Actu

func (c *client) DesiredLRPs(logger lager.Logger, filter models.DesiredLRPFilter) ([]*models.DesiredLRP, error) {
request := models.DesiredLRPsRequest{
Domain: filter.Domain,
ProcessGuids: filter.ProcessGuids,
Domain: filter.Domain,
ProcessGuids: filter.ProcessGuids,
SkipEgressRules: filter.SkipEgressRules,
Copy link
Member

Choose a reason for hiding this comment

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

I would introduce a new endpoint that only gets desired LRP routes information. Because this is not really a filter to select desired LRPs, the SkipEgressRules is a command we want to perform to present the data.

}
response := models.DesiredLRPsResponse{}
err := c.doRequest(logger, DesiredLRPsRoute_r3, nil, nil, &request, &response)
Expand Down
7 changes: 5 additions & 2 deletions handlers/desired_lrp_handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package handlers
import (
"context"
"net/http"

"code.cloudfoundry.org/auctioneer"
"code.cloudfoundry.org/bbs/db"
"code.cloudfoundry.org/bbs/events"
Expand Down Expand Up @@ -65,7 +65,7 @@ func (h *DesiredLRPHandler) commonDesiredLRPs(logger lager.Logger, targetVersion

err = parseRequest(logger, req, request)
if err == nil {
filter := models.DesiredLRPFilter{Domain: request.Domain, ProcessGuids: request.ProcessGuids}
filter := models.DesiredLRPFilter{Domain: request.Domain, ProcessGuids: request.ProcessGuids, SkipEgressRules: request.SkipEgressRules,}
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned below, I think it is better to introduce new model and new database call, that will only pull the required fields, instead of loading all of this into memory and then nullifying it.


var desiredLRPs []*models.DesiredLRP
desiredLRPs, err = h.desiredLRPDB.DesiredLRPs(req.Context(), logger, filter)
Expand All @@ -74,6 +74,9 @@ func (h *DesiredLRPHandler) commonDesiredLRPs(logger lager.Logger, targetVersion
if len(desiredLRPs[i].CachedDependencies) == 0 {
desiredLRPs[i].CachedDependencies = nil
}
if filter.SkipEgressRules {
desiredLRPs[i].EgressRules = nil
Copy link
Member

Choose a reason for hiding this comment

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

Nullifying the Egress Rules results in Desired LRP which is partially instantiated and doesn't represent the real object. Instead I think it is better to introduce new presenter model, e.g. DesiredLRPRouteInfo, that will only have information required by route-emitter.

}
}

response.DesiredLrps = desiredLRPs
Expand Down
5 changes: 3 additions & 2 deletions models/desired_lrp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ type DesiredLRPChange struct {
}

type DesiredLRPFilter struct {
Domain string
ProcessGuids []string
Domain string
ProcessGuids []string
SkipEgressRules bool
}

func PreloadedRootFS(stack string) string {
Expand Down
116 changes: 82 additions & 34 deletions models/desired_lrp_requests.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions models/desired_lrp_requests.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ message DesiredLRPsResponse {
message DesiredLRPsRequest {
string domain = 1 [(gogoproto.jsontag) = "domain"];
repeated string process_guids = 2;
bool skip_egress_rules = 3;
}

message DesiredLRPResponse {
Expand Down