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

HOSTEDCP-2075: DNS and NodePort address CEL #5164

Closed
wants to merge 4 commits into from
Closed

HOSTEDCP-2075: DNS and NodePort address CEL #5164

wants to merge 4 commits into from

Conversation

enxebre
Copy link
Member

@enxebre enxebre commented Nov 20, 2024

/hold
goes after #5155

What this PR does / why we need it:
Add cel and e2e input validation for DNS and nodepool.address

Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes #

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

@enxebre enxebre changed the title Dns and nodeport adress cel HOSTEDCP-2075: Dns and nodeport adress cel Nov 20, 2024
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 20, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 20, 2024 •
edited by openshift-ci bot
Loading

@enxebre: This pull request references HOSTEDCP-2075 which is a valid jira issue.

In response to this:

/hold
goes after #5155

What this PR does / why we need it:
Add cel and e2e input validation for DNS and nodepool.address

Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes #

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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.

@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/needs-area labels Nov 20, 2024
@openshift-ci openshift-ci bot added area/api Indicates the PR includes changes for the API area/cli Indicates the PR includes changes for CLI area/documentation Indicates the PR includes changes for documentation and removed do-not-merge/needs-area labels Nov 20, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 20, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre

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:

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

@netlify Netlify
Copy link

netlify bot commented Nov 20, 2024

✅ Deploy Preview for hypershift-docs ready!

Name Link
🔨 Latest commit 28688e8
🔍 Latest deploy log https://app.netlify.com/sites/hypershift-docs/deploys/673e01d106529c000809846f
😎 Deploy Preview https://deploy-preview-5164--hypershift-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@openshift-ci openshift-ci bot added area/testing Indicates the PR includes changes for e2e testing approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 20, 2024
@netlify Netlify
Copy link

netlify bot commented Nov 20, 2024 •
edited
Loading

✅ Deploy Preview for hypershift-docs ready!

Name Link
🔨 Latest commit a21c7a6
🔍 Latest deploy log https://app.netlify.com/sites/hypershift-docs/deploys/673f087c0cd2a20008eae53d
😎 Deploy Preview https://deploy-preview-5164--hypershift-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@enxebre
Copy link
Member Author

enxebre commented Nov 21, 2024

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 21, 2024
muraee
muraee reviewed Nov 21, 2024
View reviewed changes
api/hypershift/v1beta1/hostedcluster_types.go
@@ -808,6 +809,7 @@ type DNSSpec struct {
// It will be used to confgure ingress in the hosted cluster through the subdomain baseDomainPrefix.baseDomain.
Copy link
Contributor

@muraee muraee Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
// It will be used to confgure ingress in the hosted cluster through the subdomain baseDomainPrefix.baseDomain.
// It will be used to configure ingress in the hosted cluster through the subdomain baseDomainPrefix.baseDomain.

unrelated typo to this PR, but I think it's worth fixing here.

Copy link
Contributor

Choose a reason for hiding this comment

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

happy to tag it anyway.

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 21, 2024

@enxebre: 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
ci/prow/unit a21c7a6 link true /test unit
ci/prow/e2e-aks a21c7a6 link true /test e2e-aks

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.

@enxebre
Copy link
Member Author

enxebre commented Nov 21, 2024

/test unit

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 21, 2024

@enxebre: No presubmit jobs available for openshift/hypershift@main

In response to this:

/test unit

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.

@enxebre
Copy link
Member Author

enxebre commented Nov 21, 2024

/test unit

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 21, 2024

@enxebre: No presubmit jobs available for openshift/hypershift@main

In response to this:

/test unit

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.

@muraee
Copy link
Contributor

muraee commented Nov 21, 2024

/test unit

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Nov 21, 2024

@muraee: No presubmit jobs available for openshift/hypershift@main

In response to this:

/test unit

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.

@sjenning sjenning changed the title HOSTEDCP-2075: Dns and nodeport adress cel HOSTEDCP-2075: DNS and NodePort address CEL Nov 21, 2024
@enxebre
Copy link
Member Author

enxebre commented Nov 21, 2024 •
edited
Loading

I'm splitting this into two different PRs
#5171
#5172

@enxebre enxebre closed this Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@muraee muraee muraee left review comments

@csrwng csrwng Awaiting requested review from csrwng

@hasueki hasueki Awaiting requested review from hasueki

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/api Indicates the PR includes changes for the API area/cli Indicates the PR includes changes for CLI area/documentation Indicates the PR includes changes for documentation area/testing Indicates the PR includes changes for e2e testing jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@enxebre @openshift-ci-robot @muraee