Skip to content

Bug 2032289: omit empty fields in ConsolePlugin CR#1085

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
jotak:omitempty-ca-cert
Dec 15, 2021
Merged

Bug 2032289: omit empty fields in ConsolePlugin CR#1085
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
jotak:omitempty-ca-cert

Conversation

@jotak
Copy link
Copy Markdown
Contributor

@jotak jotak commented Dec 14, 2021

These fields are marked optional, hence should be omited when serialized
to json

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2032289

@openshift-ci openshift-ci Bot requested review from jhadvig and spadgett December 14, 2021 10:57
@jotak
Copy link
Copy Markdown
Contributor Author

jotak commented Dec 14, 2021

@jhadvig this is my first PR in openshift-api I couldn't find tests about that here, should I add some tests in console-operator instead?

@jotak jotak mentioned this pull request Dec 14, 2021
Closed
jotak added a commit to jotak/console-operator that referenced this pull request Dec 14, 2021
@jotak
Bug 2032289 - Add test case
fb54e18 
Adding a test case related to openshift/api#1085

Empty fields in ConsolePlugin Proxy , such as CACertificate, should
remain empty and not be transformed to an empty string, so that it isn't validated against regex
This was referenced Dec 14, 2021
Closed
Merged
@jotak
Copy link
Copy Markdown
Contributor Author

jotak commented Dec 14, 2021

/retest

@jotak jotak changed the title Bug 2032289 - omit empty fields in ConsolePlugin CR Bug 2032289: omit empty fields in ConsolePlugin CR Dec 14, 2021
@openshift-ci openshift-ci Bot added bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Dec 14, 2021
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 14, 2021

@jotak: This pull request references Bugzilla bug 2032289, which is invalid:

  • expected the bug to target the "4.10.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 2032289: omit empty fields in ConsolePlugin CR

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.

@jotak
Copy link
Copy Markdown
Contributor Author

jotak commented Dec 14, 2021

/bugzilla refresh

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 14, 2021

@jotak: This pull request references Bugzilla bug 2032289, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.10.0) matches configured target release for branch (4.10.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @yapei

Details

In response to this:

/bugzilla refresh

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 openshift-ci Bot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Dec 14, 2021
@openshift-ci openshift-ci Bot requested a review from yapei December 14, 2021 11:56
@sttts
Copy link
Copy Markdown
Contributor

sttts commented Dec 14, 2021

/lgtm
/approve

@sttts
Copy link
Copy Markdown
Contributor

sttts commented Dec 14, 2021

/retest

@openshift-ci openshift-ci Bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 14, 2021
@openshift-bot
Copy link
Copy Markdown

openshift-bot commented Dec 14, 2021

/retest-required

Please review the full test history for this PR and help us cut down flakes.

6 similar comments
@openshift-bot
Copy link
Copy Markdown

openshift-bot commented Dec 14, 2021

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

openshift-bot commented Dec 14, 2021

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

openshift-bot commented Dec 14, 2021

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

openshift-bot commented Dec 14, 2021

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

openshift-bot commented Dec 14, 2021

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Copy Markdown

openshift-bot commented Dec 14, 2021

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@jotak
Copy link
Copy Markdown
Contributor Author

jotak commented Dec 14, 2021

/hold

putting on hold to break the bot ci loop 😬

@openshift-bot
Copy link
Copy Markdown

openshift-bot commented Dec 14, 2021

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 14, 2021
@jotak
Bug 2032289 - omit empty fields in ConsolePlugin CR
6d5ab92 
These fields are marked optional, hence should be omited when serialized
to json
@jotak jotak force-pushed the omitempty-ca-cert branch from a5065f7 to 6d5ab92 Compare December 15, 2021 08:07
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 15, 2021
@jotak
Copy link
Copy Markdown
Contributor Author

jotak commented Dec 15, 2021

/unhold

@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 Dec 15, 2021
@jotak
Copy link
Copy Markdown
Contributor Author

jotak commented Dec 15, 2021 •
edited
Loading

@sttts I hope jobs will pass now that #1086 is merged.
So I could run make update-codegen-crds-console now, but it didn't trigger any change (i think that's expected, for a simple omitempty addition).

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 15, 2021

@jotak: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

@sttts
Copy link
Copy Markdown
Contributor

sttts commented Dec 15, 2021

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Dec 15, 2021
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 15, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jotak, sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

@openshift-merge-robot openshift-merge-robot merged commit 49df2ae into openshift:master Dec 15, 2021
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 15, 2021

@jotak: All pull requests linked via external trackers have merged:

Bugzilla bug 2032289 has been moved to the MODIFIED state.

Details

In response to this:

Bug 2032289: omit empty fields in ConsolePlugin CR

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.

jotak added a commit to jotak/console-operator that referenced this pull request Jan 14, 2022
@jotak
Bug 2032289 - Add test case
debbc44 
Adding a test case related to openshift/api#1085

Empty fields in ConsolePlugin Proxy , such as CACertificate, should
remain empty and not be transformed to an empty string, so that it isn't validated against regex
jotak added a commit to jotak/console-operator that referenced this pull request Jan 24, 2022
@jotak
Bug 2032289 - Add test case
d37c0ae 
Adding a test case related to openshift/api#1085

Empty fields in ConsolePlugin Proxy , such as CACertificate, should
remain empty and not be transformed to an empty string, so that it isn't validated against regex
jotak added a commit to jotak/console-operator that referenced this pull request Jul 15, 2022
@jotak
Bug 2032289 - Add test case
4b38508 
Adding a test case related to openshift/api#1085

Empty fields in ConsolePlugin Proxy , such as CACertificate, should
remain empty and not be transformed to an empty string, so that it isn't validated against regex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhadvig jhadvig Awaiting requested review from jhadvig

@spadgett spadgett Awaiting requested review from spadgett

@yapei yapei Awaiting requested review from yapei

Assignees

@sttts sttts

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jotak @sttts @openshift-bot @openshift-merge-robot