HIVE-2690: Soften webhook validation for Azure baseDomainResourceGroupName

[huangmingxia]

This PR relaxes the Azure webhook validation to only require baseDomainResourceGroupName when manageDNS is enabled.

[openshift-ci-robot]

@huangmingxia: This pull request references HIVE-2690 which is a valid jira issue.

In response to this:

This PR relaxes the Azure webhook validation to only require baseDomainResourceGroupName when manageDNS is enabled.

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 openshift-eng/jira-lifecycle-plugin repository.

[codecov]

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.39%. Comparing base (6104f29) to head (821e897).
⚠️ Report is 8 commits behind head on master.

Files with missing lines	Patch %	Lines
.../v1/clusterdeployment_validating_admission_hook.go	83.33%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2802      +/-   ##
==========================================
+ Coverage   50.35%   50.39%   +0.04%     
==========================================
  Files         279      279              
  Lines       34180    34190      +10     
==========================================
+ Hits        17210    17231      +21     
+ Misses      15617    15595      -22     
- Partials     1353     1364      +11     

Files with missing lines	Coverage Δ
...s/hive/v1/clusterpool_validating_admission_hook.go	80.85% <100.00%> (ø)
.../v1/clusterdeployment_validating_admission_hook.go	84.73% <83.33%> (-2.18%)	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[2uasimojo]

I'm curious why you moved this out of validateClusterPlatform?

Oh, is it because we don't currently have access to cd.Spec.ManageDNS there?

I guess the cleanest alternative is making validateClusterPlatform accept the cd instead of cd.Spec.Platform.

I can't decide whether it's worth doing that for the sake of keeping all more of the platform validation together in one spot.

This file isn't beautifully organized in the first place...

[huangmingxia]

Thank you :)
Yes, validateClusterPlatform does not have access to cd.Spec.ManageDNS, it only receives the platform fields.

I guess the cleanest alternative is making validateClusterPlatform accept the cd instead of cd.Spec.Platform.

That would certainly work, but at the time I was concerned it might change the function’s structure, so I didn’t go with that approach.

[2uasimojo]

/lgtm

I can't decide whether it's worth refactoring to try to keep more of the platform validation together. I'll leave it up to you. Unhold or re-push :)

/hold

[huangmingxia]

@2uasimojo Thanks for your insight! I’m happy to try refactoring this part :)

[openshift-ci]

@huangmingxia: 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-sigs/prow repository. I understand the commands that are listed here.

[2uasimojo]

Yup, that works!

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, huangmingxia

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:

• OWNERS [2uasimojo]

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

[huangmingxia]

/hold cancel