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.

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

APIGW: Update how status conditions for certificates are handled #17115

Merged
merged 10 commits into from
Apr 27, 2023

Conversation

jm96441n
Copy link
Member

Description

This changes how status conditions for invalid certifcates on listeners are reported so that the status condition is on the listener and mentions which certificate is invalid in the message.

Testing & Reproduction steps

Run unit tests

Links

N/A

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@jm96441n jm96441n added pr/no-docs PR does not include docs and should not trigger reminder for cherrypicking them. backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. labels Apr 24, 2023
@github-actions github-actions bot added the theme/api Relating to the HTTP API interface label Apr 24, 2023
@@ -3625,7 +4020,7 @@ func TestAPIGatewayController(t *testing.T) {
require.NoError(t, err)
ppExpected, err := json.MarshalIndent(expectedStatus, "", " ")
require.NoError(t, err)
require.True(t, statusEqual, fmt.Sprintf("statuses are unequal: %+v != %+v", string(ppActual), string(ppExpected)))
require.True(t, statusEqual, fmt.Sprintf("statuses are unequal (actual != expected): %+v != %+v", string(ppActual), string(ppExpected)))
Copy link
Member Author

Choose a reason for hiding this comment

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

this was a small QoL change, I can remove it if we think it's unnecessary

Copy link
Contributor

@andrewstucki andrewstucki left a comment

Choose a reason for hiding this comment

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

LGTM -- I do eventually want to circle back and think about whether we want to handle the Accepted status differently than we do now (since a single ResolvedRefs == False listener makes Accepted == False), but as that likely will require a decent amount of additional internal plumbing to handle appropriately, I'm cool doing it later.

@jm96441n jm96441n merged commit 391ed06 into main Apr 27, 2023
@jm96441n jm96441n deleted the NET-2029/update-status-condition-for-listeners branch April 27, 2023 15:54
sarahalsmiller pushed a commit that referenced this pull request Jun 22, 2023
)

* Move status condition for invalid certifcate to reference the listener
that is using the certificate

* Fix where we set the condition status for listeners and certificate
refs, added tests

* Add changelog
nathancoleman pushed a commit that referenced this pull request Jun 22, 2023
)

* Move status condition for invalid certifcate to reference the listener
that is using the certificate

* Fix where we set the condition status for listeners and certificate
refs, added tests

* Add changelog
sarahalsmiller pushed a commit that referenced this pull request Jun 22, 2023
…handled into release/1.15.x (#17170)

* APIGW: Update how status conditions for certificates are handled (#17115)

* Move status condition for invalid certifcate to reference the listener
that is using the certificate

* Fix where we set the condition status for listeners and certificate
refs, added tests

* Add changelog

* Remove unused code from backport

* Revert go mod changes

---------

Co-authored-by: John Maguire <john.maguire@hashicorp.com>
Co-authored-by: Nathan Coleman <nathan.coleman@hashicorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. pr/no-docs PR does not include docs and should not trigger reminder for cherrypicking them. theme/api Relating to the HTTP API interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants