Skip to content

OSDOCS#42796: Fixing the default value of macspoofchk#78823

Merged
skopacz1 merged 1 commit intoopenshift:mainfrom
jherrman:CNV-42796
Aug 6, 2024
Merged

OSDOCS#42796: Fixing the default value of macspoofchk#78823
skopacz1 merged 1 commit intoopenshift:mainfrom
jherrman:CNV-42796

Conversation

@jherrman
Copy link
Contributor

@jherrman jherrman commented Jul 12, 2024

Version(s):
4.15 and later, probably also 4.14

Issue:
https://issues.redhat.com/browse/CNV-42796

Link to docs preview:
https://78823--ocpdocs-pr.netlify.app/openshift-enterprise/latest/virt/vm_networking/virt-connecting-vm-to-linux-bridge.html

QE review:

  • QE has approved this change.

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 12, 2024
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Jul 12, 2024

Copy link

@yossisegev yossisegev left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2024
@jherrman
Copy link
Contributor Author

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Jul 24, 2024
Copy link
Contributor

@agantony agantony left a comment

Choose a reason for hiding this comment

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

Added a comment for your consideration; overall lgtm!

<4> The actual name of the Container Network Interface (CNI) plugin that provides the network for this network attachment definition. Do not change this field unless you want to use a different CNI.
<5> The name of the Linux bridge configured on the node.
<6> Optional: Flag to enable MAC spoof check. When set to `true`, you cannot change the MAC address of the pod or guest interface. This attribute provides security against a MAC spoofing attack by allowing only a single MAC address to exit the pod.
<6> Optional: A flag to enable MAC spoof check. When set to `true`, you cannot change the MAC address of the pod or guest interface. This attribute provides security against a MAC spoofing attack by allowing only a single MAC address to exit the pod.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<6> Optional: A flag to enable MAC spoof check. When set to `true`, you cannot change the MAC address of the pod or guest interface. This attribute provides security against a MAC spoofing attack by allowing only a single MAC address to exit the pod.
<6> Optional: A flag to enable the MAC spoof check. When set to `true`, you cannot change the MAC address of the pod or guest interface. This attribute provides security against a MAC spoofing attack because only a single MAC address can exit the pod.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Agil, thank you for the review! You're right about the definite article, but I would not change the final clause. The wording you suggested could imply that only a single MAC address being able to exit the pod is a general state, rather than something you config.

Thanks again,
J.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, how about a lil bit more tweaking to avoid passive voice?
s/ "A flag to enable MAC spoof check. If set to true, you cannot change the MAC address of the pod or guest interface. This attribute only lets a single MAC address to exit the pod, which provides security against a MAC spoofing attack."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, This attribute provides security against a MAC spoofing attack by allowing only a single MAC address to exit the pod. does not really contain a passive voice voice, AFAICT, but you're right that reordering the clauses makes it easier to read and comprehend. Thanks!

@agantony
Copy link
Contributor

/remove-label peer-review-in-progress
/remove-label peer-review-needed
/label peer-review-done

@openshift-ci openshift-ci bot added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Jul 24, 2024
@openshift-ci
Copy link

openshift-ci bot commented Jul 24, 2024

@agantony: Those labels are not set on the issue: peer-review-in-progress

Details

In response to this:

/remove-label peer-review-in-progress
/remove-label peer-review-needed
/label peer-review-done

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 1, 2024
@openshift-ci
Copy link

openshift-ci bot commented Aug 1, 2024

New changes are detected. LGTM label has been removed.

@jherrman
Copy link
Contributor Author

jherrman commented Aug 1, 2024

/label merge-request-needed

@openshift-ci
Copy link

openshift-ci bot commented Aug 1, 2024

@jherrman: The label(s) /label merge-request-needed cannot be applied. These labels are supported: acknowledge-critical-fixes-only, platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, no-qe, downstream-change-needed, rebase/manual, cluster-config-api-changed, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, cloud-experts, cnv, dev-tools, distributed-tracing, ims, jira/valid-bug, merge-review-in-progress, merge-review-needed, mtc, multi-arch, oadp, peer-review-done, peer-review-in-progress, peer-review-needed, rhacs, rhv, sd-docs, serverless, service-mesh, sme-review-done, sme-review-needed, staff-eng-approved, telco. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

Details

In response to this:

/label merge-request-needed

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-ci
Copy link

openshift-ci bot commented Aug 1, 2024

@jherrman: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@jherrman
Copy link
Contributor Author

jherrman commented Aug 6, 2024

/label merge-review-needed

@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label Aug 6, 2024
@skopacz1 skopacz1 added this to the Continuous Release milestone Aug 6, 2024
Copy link
Contributor

@skopacz1 skopacz1 left a comment

Choose a reason for hiding this comment

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

Merge review LGTM

@skopacz1 skopacz1 merged commit 037e865 into openshift:main Aug 6, 2024
@skopacz1
Copy link
Contributor

skopacz1 commented Aug 6, 2024

/cherrypick enterprise-4.14

@skopacz1
Copy link
Contributor

skopacz1 commented Aug 6, 2024

/cherrypick enterprise-4.15

@skopacz1
Copy link
Contributor

skopacz1 commented Aug 6, 2024

/cherrypick enterprise-4.16

@skopacz1
Copy link
Contributor

skopacz1 commented Aug 6, 2024

/cherrypick enterprise-4.17

@openshift-cherrypick-robot

@skopacz1: new pull request created: #80072

Details

In response to this:

/cherrypick enterprise-4.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-cherrypick-robot

@skopacz1: new pull request created: #80073

Details

In response to this:

/cherrypick enterprise-4.16

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-cherrypick-robot

@skopacz1: new pull request could not be created: failed to create pull request against openshift/openshift-docs#enterprise-4.15 from head openshift-cherrypick-robot:cherry-pick-78823-to-enterprise-4.15: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"No commits between openshift:enterprise-4.15 and openshift-cherrypick-robot:cherry-pick-78823-to-enterprise-4.15"}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request","status":"422"}

Details

In response to this:

/cherrypick enterprise-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-cherrypick-robot

@skopacz1: new pull request created: #80074

Details

In response to this:

/cherrypick enterprise-4.17

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@skopacz1
Copy link
Contributor

skopacz1 commented Aug 6, 2024

/cherrypick enterprise-4.15

@openshift-cherrypick-robot

@skopacz1: new pull request could not be created: failed to create pull request against openshift/openshift-docs#enterprise-4.15 from head openshift-cherrypick-robot:cherry-pick-78823-to-enterprise-4.15: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"No commits between openshift:enterprise-4.15 and openshift-cherrypick-robot:cherry-pick-78823-to-enterprise-4.15"}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request","status":"422"}

Details

In response to this:

/cherrypick enterprise-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@skopacz1
Copy link
Contributor

skopacz1 commented Aug 6, 2024

/cherrypick enterprise-4.15

@openshift-cherrypick-robot

@skopacz1: new pull request could not be created: failed to create pull request against openshift/openshift-docs#enterprise-4.15 from head openshift-cherrypick-robot:cherry-pick-78823-to-enterprise-4.15: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"No commits between openshift:enterprise-4.15 and openshift-cherrypick-robot:cherry-pick-78823-to-enterprise-4.15"}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request","status":"422"}

Details

In response to this:

/cherrypick enterprise-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@skopacz1 skopacz1 removed merge-review-in-progress Signifies that the merge review team is reviewing this PR merge-review-needed Signifies that the merge review team needs to review this PR labels Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.14 branch/enterprise-4.15 branch/enterprise-4.16 branch/enterprise-4.17 peer-review-done Signifies that the peer review team has reviewed this PR size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants