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.

Sign up for GitHub

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

Jump to bottom

Make sure the node's IP addresses is set in noProxy #5824

Merged
merged 1 commit into from
May 29, 2024
Merged

Make sure the node's IP addresses is set in noProxy #5824

merged 1 commit into from
May 29, 2024

Conversation

votdev
Copy link
Member

@votdev votdev commented May 16, 2024 •
edited

grafik

Problem:
If you configure httpProxy and httpsProxy, you must also put Harvester node's CIDR into noProxy, otherwise, the Harvester cluster will be broken.

Solution:
Add a webhook to validate the user input; it should reject if the node IP is not included in the no_proxy CIDR range.

Related Issue:
#4282

Test plan:
The test plan assumes that several hosts are available in the cluster. For the following test cases, the hosts with the following IP addresses are involved:

  • 192.168.0.30
  • 192.168.0.31
  • 192.168.0.32

Case 1:

  1. Go to Advanced > Settings and then select Edit Settings for http-proxy.
  2. Press Save.
  3. The error message noProxy should contain the node's IP addresses or CIDR. The node(s) 192.168.0.30, 192.168.0.31, 192.168.0.32 are not covered. is displayed.

grafik

Case 2:

  1. Go to Advanced > Settings and then select Edit Settings for http-proxy.
  2. Use 192.168.0.0/27 in the no-proxy form field.
  3. Press Save.
  4. The error message noProxy should contain the node's IP addresses or CIDR. The node(s) 192.168.0.32 are not covered. is displayed.

Case 3:

  1. Go to Advanced > Settings and then select Edit Settings for http-proxy.
  2. Use 192.168.0.0/24 in the no-proxy form field.
  3. Press Save.
  4. The setting is saved and you'll be redirected to the Settings page.

Case 4:

  1. Go to Advanced > Settings and then select Edit Settings for http-proxy.
  2. Use 192.168.0.30, 192.168.0.31, 192.168.0.32 in the no-proxy form field.
  3. Press Save.
  4. The setting is saved and you'll be redirected to the Settings page.

@votdev votdev added kind/enhancement Issues that improve or augment existing functionality area/admission-mutating-webhook Admission and Mutating webhooks labels May 16, 2024
@votdev votdev self-assigned this May 16, 2024
@votdev votdev force-pushed the issue_4282_noproxy branch 3 times, most recently from c43b488 to 80790ce Compare May 17, 2024 09:23
@votdev votdev marked this pull request as ready for review May 17, 2024 09:38
@Vicente-Cheng
Copy link
Contributor

Hi @votdev,
Could you move the common function (e.g. mpa/slice) to https://github.com/harvester/go-common?
That we could reuse this in various components!

Thanks!

votdev added a commit to votdev/go-common that referenced this pull request May 21, 2024
@votdev
Add several map and slice functions
378ddb1 
Relates to: harvester/harvester#5824

Signed-off-by: Volker Theile <vtheile@suse.com>
@votdev votdev mentioned this pull request May 21, 2024
Merged
@votdev
Copy link
Member Author

votdev commented May 21, 2024

Hi @votdev, Could you move the common function (e.g. mpa/slice) to harvester/go-common? That we could reuse this in various components!

Thanks!

See harvester/go-common#14

@bk201 bk201 requested review from mingshuoqiu and removed request for Vicente-Cheng May 22, 2024 02:47
@votdev votdev force-pushed the issue_4282_noproxy branch 2 times, most recently from 9477113 to cf05de8 Compare May 22, 2024 07:55
@votdev
Make sure the node's IP addresses is set in noProxy
b7dec9d 
Relates to: harvester#4282

Signed-off-by: Volker Theile <vtheile@suse.com>
@votdev votdev requested a review from Yu-Jack May 28, 2024 06:42
FrankYang0529
FrankYang0529 approved these changes May 29, 2024
View reviewed changes
Copy link
Member

@FrankYang0529 FrankYang0529 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the PR.

Yu-Jack
Yu-Jack approved these changes May 29, 2024
View reviewed changes
Copy link
Contributor

@Yu-Jack Yu-Jack left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mingshuoqiu
Copy link

LGTM.

@mingshuoqiu mingshuoqiu merged commit d06f6a7 into harvester:master May 29, 2024
11 checks passed
@votdev votdev deleted the issue_4282_noproxy branch May 29, 2024 10:52
@votdev votdev mentioned this pull request Jun 3, 2024
Merged
@votdev votdev mentioned this pull request Jun 3, 2024
Merged
@votdev
Copy link
Member Author

votdev commented Jun 3, 2024

@Mergifyio backport v1.2 v1.3

@mergify Mergify
Copy link

mergify bot commented Jun 3, 2024 •
edited

backport v1.2 v1.3

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Jun 3, 2024
@votdev @mergify
Make sure the node's IP addresses is set in noProxy (#5824)
fffcda7 
Relates to: #4282

Signed-off-by: Volker Theile <vtheile@suse.com>
(cherry picked from commit d06f6a7)
mergify bot pushed a commit that referenced this pull request Jun 3, 2024
@votdev @mergify
Make sure the node's IP addresses is set in noProxy (#5824)
29f6b46 
Relates to: #4282

Signed-off-by: Volker Theile <vtheile@suse.com>
(cherry picked from commit d06f6a7)

# Conflicts:
#	go.sum
#	pkg/webhook/resources/setting/validator_test.go
This was referenced Jun 3, 2024
Merged
Merged
votdev added a commit that referenced this pull request Jun 3, 2024
@votdev
Make sure the node's IP addresses is set in noProxy (#5824)
c1fc1c7 
Relates to: #4282

Signed-off-by: Volker Theile <vtheile@suse.com>
(cherry picked from commit d06f6a7)
votdev added a commit that referenced this pull request Jun 3, 2024
@votdev
Make sure the node's IP addresses is set in noProxy (#5824)
da4cea9 
Relates to: #4282

Signed-off-by: Volker Theile <vtheile@suse.com>
(cherry picked from commit d06f6a7)

Fixed-conflicts:
- go.sum
- pkg/webhook/resources/setting/validator_test.go
bk201 pushed a commit that referenced this pull request Jun 4, 2024
@votdev @bk201
Make sure the node's IP addresses is set in noProxy (#5824)
a1644da 
Relates to: #4282

Signed-off-by: Volker Theile <vtheile@suse.com>
(cherry picked from commit d06f6a7)

Fixed-conflicts:
- go.sum
- pkg/webhook/resources/setting/validator_test.go
FrankYang0529 pushed a commit that referenced this pull request Jun 5, 2024
@mergify @votdev
Make sure the node's IP addresses is set in noProxy (#5824) (#5936)
de1f461 
Relates to: #4282

Signed-off-by: Volker Theile <vtheile@suse.com>
(cherry picked from commit d06f6a7)

Co-authored-by: Volker Theile <vtheile@suse.com>
bk201 pushed a commit to bk201/harvester that referenced this pull request Jun 6, 2024
@mergify @votdev @bk201
Make sure the node's IP addresses is set in noProxy (harvester#5824) (h…
1a8b765 
…arvester#5936)

Relates to: harvester#4282

Signed-off-by: Volker Theile <vtheile@suse.com>
(cherry picked from commit d06f6a7)

Co-authored-by: Volker Theile <vtheile@suse.com>
bk201 pushed a commit that referenced this pull request Jun 6, 2024
@mergify @votdev @bk201
Make sure the node's IP addresses is set in noProxy (#5824) (#5936)
597e4c3 
Relates to: #4282

Signed-off-by: Volker Theile <vtheile@suse.com>
(cherry picked from commit d06f6a7)

Co-authored-by: Volker Theile <vtheile@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FrankYang0529 FrankYang0529 FrankYang0529 approved these changes

@Yu-Jack Yu-Jack Yu-Jack approved these changes

@mingshuoqiu mingshuoqiu Awaiting requested review from mingshuoqiu

Assignees

@votdev votdev

Labels
area/admission-mutating-webhook Admission and Mutating webhooks kind/enhancement Issues that improve or augment existing functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@votdev @Vicente-Cheng @mingshuoqiu @FrankYang0529 @Yu-Jack