Skip to content

[yang-models] Validating 'services' exist if ACL type is 'CTRLPLANE'#9295

Merged
ghooo merged 2 commits intosonic-net:masterfrom
ghooo:dev/mghoneim/ctrplane_services
Dec 10, 2021
Merged

[yang-models] Validating 'services' exist if ACL type is 'CTRLPLANE'#9295
ghooo merged 2 commits intosonic-net:masterfrom
ghooo:dev/mghoneim/ctrplane_services

Conversation

@ghooo
Copy link
Contributor

@ghooo ghooo commented Nov 17, 2021

Why I did it

Fixing issue #9294

How I did it

Updating ACL yang model

How to verify it

Validating issue with config patch-apply is fixed.

  • Start a KVM
  • Add file add-ctrl-plane-tbl.json-patch with content:
[
    {
     "op": "add",
     "path": "/ACL_TABLE/ACTRLPLANETABLE",
     "value": {
      "policy_desc": "ACTRLPLANETABLE",
      "services": [
       "SSH"
      ],
      "stage": "ingress",
      "type": "CTRLPLANE"
     }
    }
]
  • Run sudo config apply-patch add-ctrl-plane-tbl.json-patch

Before:

Patch Applier: The patch was sorted into 4 changes:
Patch Applier:   * [{"op": "add", "path": "/ACL_TABLE/ACTRLPLANETABLE", "value": {"type": "CTRLPLANE"}}]
Patch Applier:   * [{"op": "add", "path": "/ACL_TABLE/ACTRLPLANETABLE/policy_desc", "value": "ACTRLPLANETABLE"}]
Patch Applier:   * [{"op": "add", "path": "/ACL_TABLE/ACTRLPLANETABLE/services", "value": ["SSH"]}]
Patch Applier:   * [{"op": "add", "path": "/ACL_TABLE/ACTRLPLANETABLE/stage", "value": "ingress"}]

After:

Patch Applier: The patch was sorted into 1 change:
Patch Applier:   * [{"op": "add", "path": "/ACL_TABLE/ACTRLPLANETABLE", "value": {"policy_desc": "ACTRLPLANETABLE", "services": ["SSH"], "stage": "ingress", "type": "CTRLPLANE"}}]

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

@ghooo ghooo requested a review from lguohan as a code owner November 17, 2021 23:45
@ghooo
Copy link
Contributor Author

ghooo commented Nov 18, 2021

there is a build error, i think i will fix it and a unit-test

@dgsudharsan
Copy link
Collaborator

Can you please add UT to cover this case?

@wen587
Copy link
Contributor

wen587 commented Nov 18, 2021

Issue #9294 is fixed from my mannually test.

Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

As comment

type string;
}

must "(type != 'CTRLPLANE') or (boolean(services))";
Copy link
Collaborator

@qiluo-msft qiluo-msft Nov 18, 2021

Choose a reason for hiding this comment

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

Add some comment to explain the constrain? #Closed

Copy link
Collaborator

Choose a reason for hiding this comment

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

@bingwang-ms Do we require service field in data plane ACL?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add some unit test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@dgsudharsan dgsudharsan added the YANG YANG model related changes label Nov 18, 2021
qiluo-msft
qiluo-msft previously approved these changes Dec 9, 2021
@ghooo ghooo merged commit 02d2732 into sonic-net:master Dec 10, 2021
@qiluo-msft qiluo-msft added the Request for 202111 Branch For PRs being requested for 202111 branch label Jan 5, 2022
judyjoseph pushed a commit that referenced this pull request Jan 9, 2022
…9295)

#### Why I did it
Fixing issue #9294

#### How I did it
Updating ACL yang model

#### How to verify it

Validating issue with `config patch-apply` is fixed.

- Start a KVM
- Add file `add-ctrl-plane-tbl.json-patch ` with content:
```json
[
    {
     "op": "add",
     "path": "/ACL_TABLE/ACTRLPLANETABLE",
     "value": {
      "policy_desc": "ACTRLPLANETABLE",
      "services": [
       "SSH"
      ],
      "stage": "ingress",
      "type": "CTRLPLANE"
     }
    }
]
```
- Run `sudo config apply-patch add-ctrl-plane-tbl.json-patch`


Before:
```
Patch Applier: The patch was sorted into 4 changes:
Patch Applier:   * [{"op": "add", "path": "/ACL_TABLE/ACTRLPLANETABLE", "value": {"type": "CTRLPLANE"}}]
Patch Applier:   * [{"op": "add", "path": "/ACL_TABLE/ACTRLPLANETABLE/policy_desc", "value": "ACTRLPLANETABLE"}]
Patch Applier:   * [{"op": "add", "path": "/ACL_TABLE/ACTRLPLANETABLE/services", "value": ["SSH"]}]
Patch Applier:   * [{"op": "add", "path": "/ACL_TABLE/ACTRLPLANETABLE/stage", "value": "ingress"}]
```

After:
```
Patch Applier: The patch was sorted into 1 change:
Patch Applier:   * [{"op": "add", "path": "/ACL_TABLE/ACTRLPLANETABLE", "value": {"policy_desc": "ACTRLPLANETABLE", "services": ["SSH"], "stage": "ingress", "type": "CTRLPLANE"}}]
```

#### Which release branch to backport (provide reason below if selected)

<!--
- Note we only backport fixes to a release branch, *not* features!
- Please also provide a reason for the backporting below.
- e.g.
- [x] 202006
-->

- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106

#### Description for the changelog
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->


#### A picture of a cute animal (not mandatory but encouraged)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Included in 202111 Branch Request for 202106 Branch Request for 202111 Branch For PRs being requested for 202111 branch YANG YANG model related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants