Repository: metal3-io/baremetal-operator
Pull request #1040 (2021-11-24 08:58:22)

Validate Host DNS name in BareMetalHost resource
Validate host dns name part of BareMetalHost bmc address.

metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **hardys** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @hardys` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["hardys"]} -->
metal3-io-bot: @hroyrh: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
golint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/golint/1463759140437364736) | true | `/test golint`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1040). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:hroyrh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @hroyrh: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
golint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/golint/1463759140437364736) | true | `/test golint`
gomod | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/gomod/1463759134057828352) | true | `/test gomod`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1040). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:hroyrh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @hroyrh: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
golint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/golint/1463759140437364736) | true | `/test golint`
gomod | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/gomod/1463759134057828352) | true | `/test gomod`
manifestlint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/manifestlint/1463759144480673792) | true | `/test manifestlint`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1040). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:hroyrh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @hroyrh: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
golint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/golint/1463759140437364736) | true | `/test golint`
gomod | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/gomod/1463759134057828352) | true | `/test gomod`
manifestlint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/manifestlint/1463759144480673792) | true | `/test manifestlint`
gosec | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/gosec/1463759131906150400) | true | `/test gosec`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1040). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:hroyrh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @hroyrh: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
golint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/golint/1463759140437364736) | true | `/test golint`
gomod | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/gomod/1463759134057828352) | true | `/test gomod`
manifestlint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/manifestlint/1463759144480673792) | true | `/test manifestlint`
gosec | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/gosec/1463759131906150400) | true | `/test gosec`
gofmt | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/gofmt/1463759129658003456) | true | `/test gofmt`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1040). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:hroyrh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
namnx228: /retest-required
metal3-io-bot: @hroyrh: PR needs rebase.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
kashifest: /hold
Holding for a while. Changing the default branch reference to `main` from `master`. Once the transition is done I will unhold it. 
metal3-io-bot: @hroyrh: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
generate | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/generate/1472905481100988416) | true | `/test generate`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1040). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:hroyrh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @hroyrh: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
generate | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/generate/1472905481100988416) | true | `/test generate`
gomod | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/gomod/1472905480903856128) | true | `/test gomod`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1040). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:hroyrh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @hroyrh: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
generate | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/generate/1472905481100988416) | true | `/test generate`
gomod | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/gomod/1472905480903856128) | true | `/test gomod`
golint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/golint/1472905481121959936) | true | `/test golint`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1040). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:hroyrh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @hroyrh: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
generate | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/generate/1472905481100988416) | true | `/test generate`
gomod | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/gomod/1472905480903856128) | true | `/test gomod`
golint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/golint/1472905481121959936) | true | `/test golint`
manifestlint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/manifestlint/1472905481168097280) | true | `/test manifestlint`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1040). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:hroyrh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @hroyrh: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
generate | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/generate/1472905481100988416) | true | `/test generate`
gomod | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/gomod/1472905480903856128) | true | `/test gomod`
golint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/golint/1472905481121959936) | true | `/test golint`
manifestlint | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/manifestlint/1472905481168097280) | true | `/test manifestlint`
unit | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/unit/1472905480987742208) | true | `/test unit`
gosec | 260d5ce5b01c85f7fc41a7d82108055e365ac42e | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1040/gosec/1472905480882884608) | true | `/test gosec`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1040). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:hroyrh).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
kashifest: /hold cancel
The switch is done. BMO's default branch is now `main`
metal3-io-bot: Issues go stale after 90d of inactivity.
Mark the issue as fresh with `/remove-lifecycle stale`.
Stale issues will close after an additional 30d of inactivity.

If this issue is safe to close now please do so with `/close`.

/lifecycle stale
hroyrh: /remove-lifecycle stale

Patch:
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_validation.go b/apis/metal3.io/v1alpha1/baremetalhost_validation.go
index f2b91efc..5a64aa84 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_validation.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_validation.go
@@ -6,6 +6,7 @@ import (
 	"regexp"
 
 	"github.com/google/uuid"
+	"github.com/pkg/errors"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc"
@@ -38,6 +39,10 @@ func (host *BareMetalHost) validateHost() []error {
 		errs = append(errs, err)
 	}
 
+	if err := validateDNSName(host.Spec.BMC.Address); err != nil {
+		errs = append(errs, err)
+	}
+
 	return errs
 }
 
@@ -143,3 +148,17 @@ func validateBMHName(bmhname string) error {
 
 	return nil
 }
+
+func validateDNSName(hostaddress string) error {
+
+	if hostaddress == "" {
+		return nil
+	}
+
+	_, err := bmc.GetParsedURL(hostaddress)
+	if err != nil {
+		return errors.Wrap(err, "BMO validation")
+	}
+
+	return nil
+}


Test patch:
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_validation_test.go b/apis/metal3.io/v1alpha1/baremetalhost_validation_test.go
index 29b6b978..8b26d1b7 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_validation_test.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_validation_test.go
@@ -349,6 +349,94 @@ func TestValidateCreate(t *testing.T) {
 			oldBMH:    nil,
 			wantedErr: fmt.Sprintf("the 'numberOfPhysicalDisks'[%d] and number of 'physicalDisks'[2] is not same for volume 0", numberOfPhysicalDisks),
 		},
+		{
+			name: "validDNSName",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BareMetalHostSpec{
+					BMC: BMCDetails{
+						Address: "ipmi://host-0.example.com.org:6223"}}},
+			oldBMH:    nil,
+			wantedErr: "",
+		},
+		{
+			name: "validDNSName2",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BareMetalHostSpec{
+					BMC: BMCDetails{
+						Address: "ipmi://baremetalhost"}}},
+			oldBMH:    nil,
+			wantedErr: "",
+		},
+		{
+			name: "validDNSName3",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BareMetalHostSpec{
+					BMC: BMCDetails{
+						Address: "ipmi://[fe80::fc33:62ff:fe83:8a76]:6233"}}},
+			oldBMH:    nil,
+			wantedErr: "",
+		},
+		{
+			name: "invalidDNSNameinvalidhyphenuse",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BareMetalHostSpec{
+					BMC: BMCDetails{
+						Address: "ipmi://-host.example.com.org"}}},
+			oldBMH:    nil,
+			wantedErr: "BMO validation: failed to parse BMC address information: BMC address hostname/IP : [-host.example.com.org] is invalid",
+		},
+		{
+			name: "invalidDNSNameinvalidcharacter",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BareMetalHostSpec{
+					BMC: BMCDetails{
+						Address: "ipmi://host+1.example.com.org"}}},
+			oldBMH:    nil,
+			wantedErr: "BMO validation: failed to parse BMC address information: BMC address hostname/IP : [host+1.example.com.org] is invalid",
+		},
+		{
+			name: "invalidDNSNameinvalidformat",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BareMetalHostSpec{
+					BMC: BMCDetails{
+						Address: "[@]host.example.com"}}},
+			oldBMH:    nil,
+			wantedErr: "BMO validation: failed to parse BMC address information: parse \"ipmi://[@]host.example.com\": net/url: invalid userinfo",
+		},
+		{
+			name: "invalidDNSNameinvalidbmc",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BareMetalHostSpec{
+					BMC: BMCDetails{
+						Address: "ipm:host.example.com:6223"}}},
+			oldBMH:    nil,
+			wantedErr: "Unknown BMC type 'ipm' for address ipm:host.example.com:6223",
+		},
+		{
+			name: "invalidDNSNameinvalidipv6",
+			newBMH: &BareMetalHost{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BareMetalHostSpec{
+					BMC: BMCDetails{
+						Address: "ipmi://[fe80::fc33:62ff:fe33:8xff]:6223"}}},
+			oldBMH:    nil,
+			wantedErr: "BMO validation: failed to parse BMC address information: BMC address hostname/IP : [fe80::fc33:62ff:fe33:8xff] is invalid",
+		},
 	}
 
 	for _, tt := range tests {