MGMT-15653: Validate domain in one place #5451

CrystalChun · 2023-08-30T20:51:18Z

https://issues.redhat.com/browse/MGMT-15653
Uses domain validation function and regex
from validation package for both internal
domain validation and for domain name request
API.

Prevents mismatch in regex validation.

List all the issues related to this PR

What environments does this code impact?

Automation (CI, tools, etc)
Cloud
Operator Managed Deployments
None

How was this code tested?

assisted-test-infra environment
dev-scripts environment
Reviewer's test appreciated
Waiting for CI to do a full test run
Manual (Elaborate on how it was tested)
No tests needed

E2E KubeAPI test:

Create cluster deployment CRs specifying baseDomain: r--edht.com & baseDomain: 000.aa0
Attach agent to cluster deployment
Check that domain_name_resolution passes

Checklist

Title and description added to both, commit and PR.
Relevant issues have been associated (see CONTRIBUTING guide)
This change does not require a documentation update (docstring, docs, README, etc)
Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

Are the title and description (in both PR and commit) meaningful and clear?
Is there a bug required (and linked) for this change?
Should this PR be backported?

openshift-ci-robot · 2023-08-30T20:51:21Z

CrystalChun · 2023-08-30T20:51:29Z

/test ?

openshift-ci · 2023-08-30T20:51:52Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci · 2023-08-30T20:53:18Z

@CrystalChun: The following commands are available to trigger required jobs:

/test e2e-agent-compact-ipv4
/test edge-assisted-operator-catalog-publish-verify
/test edge-ci-index
/test edge-e2e-ai-operator-ztp
/test edge-e2e-ai-operator-ztp-sno-day2-workers
/test edge-e2e-ai-operator-ztp-sno-day2-workers-late-binding
/test edge-e2e-metal-assisted
/test edge-e2e-metal-assisted-4-10
/test edge-e2e-metal-assisted-4-11
/test edge-e2e-metal-assisted-4-12
/test edge-e2e-metal-assisted-4-9
/test edge-e2e-metal-assisted-cnv
/test edge-e2e-metal-assisted-lvm
/test edge-e2e-metal-assisted-odf
/test edge-images
/test edge-lint
/test edge-subsystem-aws
/test edge-subsystem-kubeapi-aws
/test edge-unit-test
/test edge-verify-generated-code
/test images
/test mce-images

The following commands are available to trigger optional jobs:

/test e2e-agent-ha-dualstack
/test e2e-agent-sno-ipv6
/test edge-e2e-ai-operator-ztp-3masters
/test edge-e2e-ai-operator-ztp-capi
/test edge-e2e-ai-operator-ztp-compact-day2-masters
/test edge-e2e-ai-operator-ztp-compact-day2-workers
/test edge-e2e-ai-operator-ztp-disconnected
/test edge-e2e-ai-operator-ztp-hypershift-zero-nodes
/test edge-e2e-ai-operator-ztp-multiarch-3masters-ocp-411
/test edge-e2e-ai-operator-ztp-multiarch-sno-ocp-411
/test edge-e2e-ai-operator-ztp-node-labels
/test edge-e2e-ai-operator-ztp-sno-day2-masters
/test edge-e2e-ai-operator-ztp-sno-day2-workers-ignitionoverride
/test edge-e2e-metal-assisted-4-13
/test edge-e2e-metal-assisted-4-14
/test edge-e2e-metal-assisted-cnv-4.14
/test edge-e2e-metal-assisted-day2
/test edge-e2e-metal-assisted-day2-arm-workers
/test edge-e2e-metal-assisted-day2-single-node
/test edge-e2e-metal-assisted-ipv4v6
/test edge-e2e-metal-assisted-ipv6
/test edge-e2e-metal-assisted-kube-api-late-binding-single-node
/test edge-e2e-metal-assisted-kube-api-late-unbinding-ipv4-single-node
/test edge-e2e-metal-assisted-kube-api-net-suite
/test edge-e2e-metal-assisted-mce
/test edge-e2e-metal-assisted-mce-4-10
/test edge-e2e-metal-assisted-mce-sno
/test edge-e2e-metal-assisted-metallb
/test edge-e2e-metal-assisted-none
/test edge-e2e-metal-assisted-odf-4-14
/test edge-e2e-metal-assisted-onprem
/test edge-e2e-metal-assisted-single-node
/test edge-e2e-metal-assisted-static-ip-suite
/test edge-e2e-metal-assisted-tang
/test edge-e2e-metal-assisted-tpmv2
/test edge-e2e-metal-assisted-upgrade-agent
/test edge-e2e-nutanix-assisted
/test edge-e2e-nutanix-assisted-2workers
/test edge-e2e-oci-assisted
/test edge-e2e-vsphere-assisted
/test edge-e2e-vsphere-assisted-4-12
/test edge-e2e-vsphere-assisted-4-13
/test edge-e2e-vsphere-assisted-umn
/test edge-push-pr-image
/test push-pr-image

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-assisted-service-master-e2e-agent-compact-ipv4
pull-ci-openshift-assisted-service-master-edge-assisted-operator-catalog-publish-verify
pull-ci-openshift-assisted-service-master-edge-ci-index
pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-ztp
pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-ztp-capi
pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-ztp-hypershift-zero-nodes
pull-ci-openshift-assisted-service-master-edge-e2e-metal-assisted
pull-ci-openshift-assisted-service-master-edge-images
pull-ci-openshift-assisted-service-master-edge-lint
pull-ci-openshift-assisted-service-master-edge-subsystem-aws
pull-ci-openshift-assisted-service-master-edge-subsystem-kubeapi-aws
pull-ci-openshift-assisted-service-master-edge-unit-test
pull-ci-openshift-assisted-service-master-edge-verify-generated-code
pull-ci-openshift-assisted-service-master-images
pull-ci-openshift-assisted-service-master-mce-images

In response to this:

/test ?

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.

CrystalChun · 2023-08-30T20:54:40Z

/test edge-e2e-metal-assisted
/test edge-e2e-ai-operator-ztp

openshift-ci-robot · 2023-08-30T23:47:37Z

openshift-ci-robot · 2023-08-31T14:57:34Z

CrystalChun · 2023-08-31T15:00:31Z

/cc @filanov @carbonin @ori-amizur

codecov · 2023-08-31T15:16:42Z

Codecov Report

Merging #5451 (6e6dcc2) into master (6beee21) will increase coverage by 0.11%.
Report is 4 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5451      +/-   ##
==========================================
+ Coverage   67.73%   67.84%   +0.11%     
==========================================
  Files         229      232       +3     
  Lines       33571    34339     +768     
==========================================
+ Hits        22738    23297     +559     
- Misses       8791     8976     +185     
- Partials     2042     2066      +24

see 5 files with indirect coverage changes

gamli75 · 2023-09-03T12:36:25Z

@CrystalChun can you add tests to cover the problem that we had?

gamli75 · 2023-09-04T11:09:08Z

/retest-required

https://issues.redhat.com/browse/MGMT-15653 Uses domain validation function and regex from validation package for both internal domain validation and for domain name request API. Prevents mismatch in regex validation.

https://issues.redhat.com/browse/MGMT-15653 After running skipper make generate

CrystalChun · 2023-09-05T22:44:09Z

@CrystalChun can you add tests to cover the problem that we had?

Test cases added:
Unit test for domain starting with numbers
https://github.com/openshift/assisted-service/pull/5451/files#diff-8e49f4c68fa47a30fe1298514f665ab41f5c860506f6268d971873b0cd94ef35R243-R246

Subsystem tests added for domains containing a dash, containing multiple dashes, starting with a number, and ending with a number:
https://github.com/openshift/assisted-service/pull/5451/files#diff-592a1c4ce8671b4da6257edd65ba0164de5a751efada778cf22c9cee33084ec6R1049-R1068

Let me know if there are more test cases to cover!

gamli75 · 2023-09-06T07:34:24Z

/retest-required

CrystalChun · 2023-09-06T14:26:51Z

ci/prow/e2e-agent-compact-ipv4 test is failing due to known issue from agent installer team and can be skipped

https://redhat-internal.slack.com/archives/C02SPBZ4GPR/p1694010302616379?thread_ts=1694009504.979739&cid=C02SPBZ4GPR

osherdp · 2023-09-06T14:27:06Z

/override ci/prow/e2e-agent-compact-ipv4

openshift-ci · 2023-09-06T14:27:25Z

@osherdp: Overrode contexts on behalf of osherdp: ci/prow/e2e-agent-compact-ipv4

In response to this:

/override ci/prow/e2e-agent-compact-ipv4

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 · 2023-09-06T14:27:27Z

@CrystalChun: 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
ci/prow/edge-e2e-ai-operator-ztp-hypershift-zero-nodes	`6e6dcc2`	link	false	`/test edge-e2e-ai-operator-ztp-hypershift-zero-nodes`

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 · 2023-09-06T20:42:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CrystalChun, gamli75

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~~ [CrystalChun,gamli75]

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

* MGMT-15653: Validate domain in one place https://issues.redhat.com/browse/MGMT-15653 Uses domain validation function and regex from validation package for both internal domain validation and for domain name request API. Prevents mismatch in regex validation. * MGMT-15653: Generated files https://issues.redhat.com/browse/MGMT-15653 After running skipper make generate

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 30, 2023

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 30, 2023

openshift-ci bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 30, 2023

openshift-ci bot added api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 30, 2023

CrystalChun force-pushed the dns branch 2 times, most recently from ded5142 to ab2d111 Compare August 31, 2023 14:56

CrystalChun marked this pull request as ready for review August 31, 2023 14:57

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 31, 2023

openshift-ci bot requested review from eranco74 and rccrdpccl August 31, 2023 14:58

openshift-ci bot requested review from carbonin, filanov and ori-amizur August 31, 2023 15:00

CrystalChun force-pushed the dns branch 4 times, most recently from 02c6e86 to 0ee6d25 Compare September 5, 2023 22:36

MGMT-15653: Validate domain in one place

e8c07b9

https://issues.redhat.com/browse/MGMT-15653 Uses domain validation function and regex from validation package for both internal domain validation and for domain name request API. Prevents mismatch in regex validation.

MGMT-15653: Generated files

6e6dcc2

https://issues.redhat.com/browse/MGMT-15653 After running skipper make generate

CrystalChun force-pushed the dns branch from 0ee6d25 to 6e6dcc2 Compare September 5, 2023 22:41

gamli75 approved these changes Sep 6, 2023

View reviewed changes

openshift-ci bot assigned gamli75 Sep 6, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 6, 2023

openshift-merge-robot merged commit e235675 into openshift:master Sep 7, 2023
16 of 17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MGMT-15653: Validate domain in one place #5451

MGMT-15653: Validate domain in one place #5451

CrystalChun commented Aug 30, 2023 •

edited

openshift-ci-robot commented Aug 30, 2023 •

edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

CrystalChun commented Aug 30, 2023

openshift-ci bot commented Aug 30, 2023

openshift-ci bot commented Aug 30, 2023

CrystalChun commented Aug 30, 2023

openshift-ci-robot commented Aug 30, 2023 •

edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

openshift-ci-robot commented Aug 31, 2023 •

edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

CrystalChun commented Aug 31, 2023

codecov bot commented Aug 31, 2023 •

edited

gamli75 commented Sep 3, 2023

gamli75 commented Sep 4, 2023

CrystalChun commented Sep 5, 2023

gamli75 commented Sep 6, 2023

CrystalChun commented Sep 6, 2023

osherdp commented Sep 6, 2023

openshift-ci bot commented Sep 6, 2023

openshift-ci bot commented Sep 6, 2023

openshift-ci bot commented Sep 6, 2023

MGMT-15653: Validate domain in one place #5451

MGMT-15653: Validate domain in one place #5451

Conversation

CrystalChun commented Aug 30, 2023 • edited

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

openshift-ci-robot commented Aug 30, 2023 • edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

CrystalChun commented Aug 30, 2023

openshift-ci bot commented Aug 30, 2023

openshift-ci bot commented Aug 30, 2023

CrystalChun commented Aug 30, 2023

openshift-ci-robot commented Aug 30, 2023 • edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

openshift-ci-robot commented Aug 31, 2023 • edited by openshift-ci bot

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

CrystalChun commented Aug 31, 2023

codecov bot commented Aug 31, 2023 • edited

Codecov Report

gamli75 commented Sep 3, 2023

gamli75 commented Sep 4, 2023

CrystalChun commented Sep 5, 2023

gamli75 commented Sep 6, 2023

CrystalChun commented Sep 6, 2023

osherdp commented Sep 6, 2023

openshift-ci bot commented Sep 6, 2023

openshift-ci bot commented Sep 6, 2023

openshift-ci bot commented Sep 6, 2023

CrystalChun commented Aug 30, 2023 •

edited

openshift-ci-robot commented Aug 30, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Aug 30, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Aug 31, 2023 •

edited by openshift-ci bot

codecov bot commented Aug 31, 2023 •

edited