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

libnetwork: loosen container IPAM validation #47132

Merged
merged 1 commit into from
Jan 20, 2024

Conversation

corhere
Copy link
Contributor

@corhere corhere commented Jan 20, 2024

- What I did
Permit container network attachments to set any static IP address within the network's IPAM master pool, including when a subpool is configured. Users have come to depend on being able to statically assign container IP addresses which are guaranteed not to collide with automatically- assigned container addresses.

- How I did it
By deleting a couple lines of code

- How to verify it
New regression test

- Description for the changelog

  • Fixed a regression which restricted static container IP address assignments to the --ip-range of a container network

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

Permit container network attachments to set any static IP address within
the network's IPAM master pool, including when a subpool is configured.
Users have come to depend on being able to statically assign container
IP addresses which are guaranteed not to collide with automatically-
assigned container addresses.

Signed-off-by: Cory Snider <csnider@mirantis.com>
@@ -289,3 +289,40 @@ func TestMacAddressIsAppliedToMainNetworkWithShortID(t *testing.T) {
c := container.Inspect(ctx, t, apiClient, cid)
assert.Equal(t, c.NetworkSettings.Networks["testnet"].MacAddress, "02:42:08:26:a9:55")
}

func TestStaticIPOutsideSubpool(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it's useful to add a description to the test (based on your PR description on GitHub); this seems like one of those cases where context may be useful to know "why" this test is.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM (left a suggestion for the test)

Copy link
Contributor

@robmry robmry left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit 17c3829 into moby:master Jan 20, 2024
126 checks passed
@thaJeztah
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can no longer set an IP address inside of a subnet range when subnet range is larger than IP range
3 participants