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.

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

OCPBUGS-22749: Adjust NAD name by using unique-names-generator #13263

Conversation

hstastna
Copy link
Contributor

@hstastna hstastna commented Oct 20, 2023

Fixes:
https://issues.redhat.com/browse/OCPBUGS-22749

Analysis / Root cause:
By using the "Create Network Attachment Definition" dialog, an administrator may create two NADs in two different namespaces that would be connected to a single L2 network. There is nothing in the UI that indicates that namespace isolation is breached.

Solution Description:
Adjust the NetworkAttachmentDefinition name after its creation to make sure the name will be unique, to prevent creating a network shared between isolated namespaces. Use unique-names-generator package to generate the unique names that are appended to the name filled in the form for creating NADs.

Screen shots / Gifs for design review:
Before:
NADs with same names, shared NADs between the different namespaces can be created:
nad_before

After:
Unique NADs names:
nad_after

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 20, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 20, 2023

@hstastna: This pull request references CNV-32926 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.15.0" version, but no target version was set.

In response to this:

Fixes:
https://issues.redhat.com/browse/CNV-32926

Analysis / Root cause:
By using the "Create Network Attachment Definition" dialog, an administrator may create two NADs in two different namespaces that would be connected to a single L2 network. There is nothing in the UI that indicates that namespace isolation is breached.

Solution Description:
Adjust the NetworkAttachmentDefinition name after its creation to make sure the name will be unique, to prevent creating a network shared between isolated namespaces. Use unique-names-generator package to generate the unique names that are appended to the name filled in the form for creating NADs.

Screen shots / Gifs for design review:
Before:
NADs with same names, shared NADs between the different namespaces can be created:
nad_before

After:
Unique NADs names:
nad_after

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/test-infra repository.

@openshift-ci openshift-ci bot added the component/network-attachment-definition Related to network-attachment-definition label Oct 20, 2023
@hstastna hstastna force-pushed the BZ_Admin_can_create_network_shared_between_ns branch from 40b0edc to 103aa9b Compare October 20, 2023 10:18
@hstastna
Copy link
Contributor Author

@avivtur @metalice @pcbailey @upalatucci @vojtechszocs please review

1 similar comment
@hstastna
Copy link
Contributor Author

@avivtur @metalice @pcbailey @upalatucci @vojtechszocs please review

@metalice
Copy link
Contributor

i can see it already installed by another package, you can just import it without installing it with yarn add.
image

@hstastna
Copy link
Contributor Author

hstastna commented Oct 24, 2023

i can see it already installed by another package, you can just import it without installing it with yarn add. image

Well, it didn't work when I tried to just import it. It acted like the package didn't exist. Did you try to just import it?

@metalice
Copy link
Contributor

hmm tried to import.. didn't get any errors,
image

@metalice
Copy link
Contributor

you are adding to fornend/package.json and its has
image

Adjust the NetworkAttachmentDefinition name after its creation to make
sure the name will be unique, to prevent creating a network shared
between isolated namespaces.

Fixes https://issues.redhat.com/browse/CNV-32926
@hstastna hstastna force-pushed the BZ_Admin_can_create_network_shared_between_ns branch from 103aa9b to 6e77a4a Compare October 24, 2023 15:58
@hstastna
Copy link
Contributor Author

@metalice I've played with the code and it started to work even without adding the package anywhere. Weird that it didn't work previously.

@metalice
Copy link
Contributor

/retest

2 similar comments
@metalice
Copy link
Contributor

/retest

@metalice
Copy link
Contributor

/retest

@hstastna
Copy link
Contributor Author

@avivtur @metalice @pcbailey @upalatucci @vojtechszocs please review

@pcbailey
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 26, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hstastna, pcbailey

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 26, 2023
@hstastna
Copy link
Contributor Author

/jira refresh

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 31, 2023

@hstastna: This pull request references CNV-32926 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.15.0" version, but no target version was set.

In response to this:

/jira refresh

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/test-infra repository.

@hstastna
Copy link
Contributor Author

/jira refresh

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 31, 2023

@hstastna: This pull request references CNV-32926 which is a valid jira issue.

In response to this:

/jira refresh

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/test-infra repository.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 31, 2023

@hstastna: This pull request references CNV-32926 which is a valid jira issue.

In response to this:

Fixes:
https://issues.redhat.com/browse/OCPBUGS-22749

Analysis / Root cause:
By using the "Create Network Attachment Definition" dialog, an administrator may create two NADs in two different namespaces that would be connected to a single L2 network. There is nothing in the UI that indicates that namespace isolation is breached.

Solution Description:
Adjust the NetworkAttachmentDefinition name after its creation to make sure the name will be unique, to prevent creating a network shared between isolated namespaces. Use unique-names-generator package to generate the unique names that are appended to the name filled in the form for creating NADs.

Screen shots / Gifs for design review:
Before:
NADs with same names, shared NADs between the different namespaces can be created:
nad_before

After:
Unique NADs names:
nad_after

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/test-infra repository.

@hstastna hstastna changed the title CNV-32926: Adjust NAD name by using unique-names-generator OCPBUGS-22749: Adjust NAD name by using unique-names-generator Oct 31, 2023
@openshift-ci-robot openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Oct 31, 2023
@openshift-ci-robot
Copy link
Contributor

@hstastna: This pull request references Jira Issue OCPBUGS-22749, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.15.0) matches configured target version for branch (4.15.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira (ycui@redhat.com), skipping review request.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Fixes:
https://issues.redhat.com/browse/OCPBUGS-22749

Analysis / Root cause:
By using the "Create Network Attachment Definition" dialog, an administrator may create two NADs in two different namespaces that would be connected to a single L2 network. There is nothing in the UI that indicates that namespace isolation is breached.

Solution Description:
Adjust the NetworkAttachmentDefinition name after its creation to make sure the name will be unique, to prevent creating a network shared between isolated namespaces. Use unique-names-generator package to generate the unique names that are appended to the name filled in the form for creating NADs.

Screen shots / Gifs for design review:
Before:
NADs with same names, shared NADs between the different namespaces can be created:
nad_before

After:
Unique NADs names:
nad_after

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/test-infra repository.

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 932bbf4 and 2 for PR HEAD 6e77a4a in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 08bc0e7 and 1 for PR HEAD 6e77a4a in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 7261467 and 0 for PR HEAD 6e77a4a in total

@openshift-ci-robot
Copy link
Contributor

/hold

Revision 6e77a4a was retested 3 times: holding

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 1, 2023
@hstastna
Copy link
Contributor Author

hstastna commented Nov 1, 2023

/retest

@hstastna
Copy link
Contributor Author

hstastna commented Nov 1, 2023

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 1, 2023
@hstastna
Copy link
Contributor Author

hstastna commented Nov 1, 2023

/retest

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 7261467 and 2 for PR HEAD 6e77a4a in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 4dc716b and 1 for PR HEAD 6e77a4a in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD b5d952e and 0 for PR HEAD 6e77a4a in total

@openshift-ci-robot
Copy link
Contributor

/hold

Revision 6e77a4a was retested 3 times: holding

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 2, 2023
@hstastna
Copy link
Contributor Author

hstastna commented Nov 3, 2023

/hold cancel
/retest

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 3, 2023
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD af5b08f and 2 for PR HEAD 6e77a4a in total

Copy link
Contributor

openshift-ci bot commented Nov 4, 2023

@hstastna: all tests passed!

Full PR test history. Your PR dashboard.

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/test-infra repository. I understand the commands that are listed here.

@openshift-ci openshift-ci bot merged commit bde3ee8 into openshift:master Nov 4, 2023
6 checks passed
@openshift-ci-robot
Copy link
Contributor

@hstastna: Jira Issue OCPBUGS-22749: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-22749 has been moved to the MODIFIED state.

In response to this:

Fixes:
https://issues.redhat.com/browse/OCPBUGS-22749

Analysis / Root cause:
By using the "Create Network Attachment Definition" dialog, an administrator may create two NADs in two different namespaces that would be connected to a single L2 network. There is nothing in the UI that indicates that namespace isolation is breached.

Solution Description:
Adjust the NetworkAttachmentDefinition name after its creation to make sure the name will be unique, to prevent creating a network shared between isolated namespaces. Use unique-names-generator package to generate the unique names that are appended to the name filled in the form for creating NADs.

Screen shots / Gifs for design review:
Before:
NADs with same names, shared NADs between the different namespaces can be created:
nad_before

After:
Unique NADs names:
nad_after

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/test-infra repository.

@openshift-merge-robot
Copy link
Contributor

Fix included in accepted release 4.15.0-0.nightly-2023-11-04-080954

@openshift-merge-robot
Copy link
Contributor

Fix included in accepted release 4.15.0-0.nightly-2024-02-12-213938

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/network-attachment-definition Related to network-attachment-definition jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants