Skip to content

Commit

Permalink
XRevert "adding new set of test cases for secgroups related to deny r…
Browse files Browse the repository at this point in the history 
…ule changes"

This reverts commit f895efb.
  • Loading branch information
bashokba committed Sep 11, 2020
1 parent f895efb commit 85e302c
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 119 deletions.
13 changes: 5 additions & 8 deletions agent-ovs/lib/PolicyManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1067,7 +1067,6 @@ static bool updatePolicyRules(OFFramework& framework,
using modelgbp::gbp::AllowDenyAction;
using modelgbp::gbp::RedirectAction;
using modelgbp::gbp::RedirectDestGroup;
using modelgbp::gbp::LogAction;

optional<shared_ptr<Parent> > parent =
Parent::resolve(framework, parentURI);
Expand Down Expand Up @@ -1158,14 +1157,12 @@ static bool updatePolicyRules(OFFramework& framework,
RedirectDestGroup::resolve(framework, destGrpUri.get());
newRedirGrps.insert(destGrpUri.get());
}
else if(r->getTargetClass().get() == LogAction::CLASS_ID) {
optional<shared_ptr<modelgbp::gbp::LogAction> > resloveLog =
modelgbp::gbp::LogAction::resolve(framework,r->getTargetURI().get());
if (resloveLog) {
ruleLog = resloveLog.get()->getLog(0) != 0 ;
}
optional<shared_ptr<modelgbp::gbp::LogAction> > resloveLog =
modelgbp::gbp::LogAction::resolve(framework,r->getTargetURI().get());
if (resloveLog) {
ruleLog = resloveLog.get()->getLog(0) != 0 ;
}
}
}

uint16_t clsPrio = 0;
for (const shared_ptr<L24Classifier>& c : classifiers) {
Expand Down
3 changes: 2 additions & 1 deletion agent-ovs/lib/include/opflexagent/test/ModbFixture.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ class ModbFixture : public BaseFixture {
std::shared_ptr<modelgbp::gbpe::L24Classifier> classifier10;

std::shared_ptr<modelgbp::gbp::AllowDenyAction> action1;
std::shared_ptr<modelgbp::gbp::LogAction> action2;
std::shared_ptr<modelgbp::gbp::LogAction> log1;

std::shared_ptr<modelgbp::gbp::Contract> con1;
std::shared_ptr<modelgbp::gbp::Contract> con2;
std::shared_ptr<modelgbp::gbp::Contract> con3;
Expand Down
1 change: 1 addition & 0 deletions agent-ovs/ovs/AccessFlowManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,7 @@ void AccessFlowManager::handleSecGrpSetUpdate(const uri_set_t& secGrps,
}

uint32_t secGrpSetId = idGen.getId(ID_NMSPC_SECGROUP_SET, secGrpsIdStr);

FlowEntryList secGrpIn;
FlowEntryList secGrpOut;

Expand Down
12 changes: 4 additions & 8 deletions agent-ovs/ovs/FlowUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,10 @@ void add_l2classifier_entries(L24Classifier& clsfr, ClassAction act, bool log,
if (act != flowutils::CA_DENY)
f.action().go(nextTable);
if (act == flowutils::CA_DENY) {
if (log != 0) {
if (log != 0)
f.action().dropLog(currentTable,ActionBuilder::CaptureReason::POLICY_DENY).go(nextTable);
}
else {
else
f.action().metadata(0, flow::meta::DROP_LOG).go(nextTable);
}
}
entries.push_back(f.build());
}
Expand Down Expand Up @@ -268,12 +266,10 @@ void add_classifier_entries(L24Classifier& clsfr, ClassAction act, bool log,

switch (act) {
case flowutils::CA_DENY:
if (log != 0) {
if (log != 0)
f.action().dropLog(currentTable,ActionBuilder::CaptureReason::POLICY_DENY).go(nextTable);
}
else {
else
f.action().metadata(0, flow::meta::DROP_LOG).go(nextTable);
}
case flowutils::CA_ALLOW:
case flowutils::CA_REFLEX_FWD_TRACK:
case flowutils::CA_REFLEX_FWD:
Expand Down
120 changes: 18 additions & 102 deletions agent-ovs/ovs/test/AccessFlowManager_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,12 @@ class AccessFlowManagerFixture : public FlowManagerFixture {
uint16_t initExpSecGrp2(uint32_t setId);
void initExpSecGrpSet1();
void initExpSecGrpSet12(bool second = true, int remoteAddress = 0);
uint16_t initExpSecGrp3(int remoteAddress);

AccessFlowManager accessFlowManager;

shared_ptr<SecGroup> secGrp1;
shared_ptr<SecGroup> secGrp2;
shared_ptr<SecGroup> secGrp3;

/* Initialize dhcp flow entries */
void initExpDhcpEp(shared_ptr<Endpoint>& ep);

Expand Down Expand Up @@ -196,99 +195,6 @@ enum CaptureReason {
POLICY_PERMIT=2
};

uint16_t AccessFlowManagerFixture::initExpSecGrp3(int remoteAddress) {

uint32_t setId = 2;
uint16_t prio = PolicyManager::MAX_POLICY_RULE_PRIORITY;
PolicyManager::rule_list_t rules;
agent.getPolicyManager().getSecGroupRules(secGrp3->getURI(), rules);
uint32_t ruleId;
/* classifer 2 */
ruleId = idGen.getId("l24classifierRule", classifier2->getURI().toString());
if (remoteAddress) {
ADDF(Bldr(SEND_FLOW_REM).table(OUT_POL).priority(prio).cookie(ruleId)
.arp().reg(SEPG, setId).isTpa("192.169.0.0/16").actions().dropLog(OUT_POL, POLICY_DENY).go(EXP_DROP).done());
}
/* classifer 1 */
ruleId = idGen.getId("l24classifierRule", classifier1->getURI().toString());
if (remoteAddress) {
ADDF(Bldr(SEND_FLOW_REM).table(IN_POL).priority(prio-128).cookie(ruleId)
.tcp().reg(SEPG, setId).isIpSrc("192.169.0.0/16").isTpDst(80).actions().dropLog(IN_POL, POLICY_DENY).go(EXP_DROP).done());
}

return 512;
}

BOOST_FIXTURE_TEST_CASE(denyrule, AccessFlowManagerFixture) {
createObjects();
createPolicyObjects();
shared_ptr<modelgbp::gbp::Subnets> rs1;
{
Mutator mutator(framework, "policyreg");
rs1 = space->addGbpSubnets("subnets_rule_1");
rs1->addGbpSubnet("subnets_rule1_1")
->setAddress("192.169.0.0")
.setPrefixLen(16);

shared_ptr<modelgbp::gbp::SecGroupRule> r1, r2, r3, r4;
//secgrp 3
secGrp3 = space->addGbpSecGroup("secgrp3");
//action 1
action1 = space->addGbpAllowDenyAction("action1");
action1->setAllow(0).setOrder(5);
action2 = space->addGbpLogAction("action2");
action2->setLog(1);
//security group rule
r1 = secGrp3->addGbpSecGroupSubject("1_subject1")
->addGbpSecGroupRule("1_1_rule1");
r1->setDirection(DirectionEnumT::CONST_OUT).setOrder(100)
.addGbpRuleToClassifierRSrc(classifier2->getURI().toString());
r1->addGbpSecGroupRuleToRemoteAddressRSrc(rs1->getURI().toString());
r1->addGbpRuleToActionRSrcAllowDenyAction(action1->getURI().toString())
->setTargetAllowDenyAction(action1->getURI());
r1->addGbpRuleToActionRSrcLogAction(action2->getURI().toString())
->setTargetLogAction(action2->getURI());


r2 = secGrp3->addGbpSecGroupSubject("1_subject1")
->addGbpSecGroupRule("1_1_rule2");
r2->setDirection(DirectionEnumT::CONST_IN).setOrder(150)
.addGbpRuleToClassifierRSrc(classifier1->getURI().toString());
r2->addGbpSecGroupRuleToRemoteAddressRSrc(rs1->getURI().toString());
r2->addGbpRuleToActionRSrcAllowDenyAction(action1->getURI().toString())
->setTargetAllowDenyAction(action1->getURI());
r2->addGbpRuleToActionRSrcLogAction(action2->getURI().toString())
->setTargetLogAction(action2->getURI());

r3 = secGrp3->addGbpSecGroupSubject("1_subject1")
->addGbpSecGroupRule("1_1_rule3");
r3->setDirection(DirectionEnumT::CONST_BIDIRECTIONAL).setOrder(200)
.addGbpRuleToClassifierRSrc(classifier5->getURI().toString());
r3->addGbpSecGroupRuleToRemoteAddressRSrc(rs1->getURI().toString());
r3->addGbpRuleToActionRSrcAllowDenyAction(action1->getURI().toString())
->setTargetAllowDenyAction(action1->getURI());
r3->addGbpRuleToActionRSrcLogAction(action2->getURI().toString())
->setTargetLogAction(action2->getURI());

mutator.commit();
}

ep0.reset(new Endpoint("0-0-0-0"));
epSrc.updateEndpoint(*ep0);

initExpStatic();
WAIT_FOR_TABLES("empty-secgrp", 500);

ep0->addSecurityGroup(opflex::modb::URI("/PolicyUniverse/PolicySpace"
"/tenant0/GbpSecGroup/secgrp3/"));
epSrc.updateEndpoint(*ep0);

clearExpFlowTables();
initExpStatic();

initExpSecGrp3(1);
WAIT_FOR_TABLES("deny-rule", 500);
}

BOOST_FIXTURE_TEST_CASE(secGrp, AccessFlowManagerFixture) {
createObjects();
Expand All @@ -308,6 +214,11 @@ BOOST_FIXTURE_TEST_CASE(secGrp, AccessFlowManagerFixture) {
shared_ptr<modelgbp::gbp::SecGroupRule> r1, r2, r3, r4, r5 ;
secGrp1 = space->addGbpSecGroup("secgrp1");

action1 = space->addGbpAllowDenyAction("action1");
action1->setAllow(0).setOrder(5);
log1 = space->addGbpLogAction("log1");
log1->setLog(1);

r1 = secGrp1->addGbpSecGroupSubject("1_subject1")
->addGbpSecGroupRule("1_1_rule1");
r1->setDirection(DirectionEnumT::CONST_IN).setOrder(100)
Expand Down Expand Up @@ -414,8 +325,14 @@ BOOST_FIXTURE_TEST_CASE(secGrp, AccessFlowManagerFixture) {

secGrp1->addGbpSecGroupSubject("1_subject1")
->addGbpSecGroupRule("1_1_rule3")
->addGbpSecGroupRuleToRemoteAddressRSrc(rs->getURI().toString());
->addGbpSecGroupRuleToRemoteAddressRSrc(rs->getURI().toString());

secGrp1->addGbpSecGroupSubject("1_subject1")
->addGbpSecGroupRule("1_1_rule3")
->addGbpRuleToActionRSrcAllowDenyAction(action1->getURI().toString());
secGrp1->addGbpSecGroupSubject("1_subject1")
->addGbpSecGroupRule("1_1_rule3")
->addGbpRuleToActionRSrcLogAction(log1->getURI().toString());
mutator.commit();
}
clearExpFlowTables();
Expand Down Expand Up @@ -622,12 +539,11 @@ uint16_t AccessFlowManagerFixture::initExpSecGrp1(uint32_t setId,
ruleId = idGen.getId("l24classifierRule",
classifier2->getURI().toString());
if (remoteAddress) {
ADDF(Bldr(SEND_FLOW_REM).table(OUT_POL).priority(prio-256).cookie(ruleId)
.arp().reg(SEPG, setId).isTpa("192.168.0.0/16").actions().go(OUT).done());
if (remoteAddress > 1)
ADDF(Bldr(SEND_FLOW_REM).table(OUT_POL).priority(prio-256).cookie(ruleId)
.arp().reg(SEPG, setId).isTpa("10.0.0.0/8").actions()
.go(OUT).done());
ADDF(Bldr(SEND_FLOW_REM).table(OUT_POL).priority(prio-256).cookie(ruleId)
.arp().reg(SEPG, setId).isTpa("192.168.0.0/16").actions().dropLog(OUT_POL, POLICY_DENY).go(EXP_DROP).done());
if (remoteAddress > 1)
ADDF(Bldr(SEND_FLOW_REM).table(OUT_POL).priority(prio-256).cookie(ruleId)
.arp().reg(SEPG, setId).isTpa("10.0.0.0/8").actions().dropLog(OUT_POL, POLICY_DENY).go(EXP_DROP).done());
} else {
ADDF(Bldr(SEND_FLOW_REM).table(OUT_POL).priority(prio-256).cookie(ruleId)
.arp().reg(SEPG, setId).actions().go(OUT).done());
Expand Down

0 comments on commit 85e302c

Please sign in to comment.