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

Remove mTLS from state that returns 404 #794

Merged
merged 9 commits into from
Jan 3, 2024
Merged

Remove mTLS from state that returns 404 #794

merged 9 commits into from
Jan 3, 2024

Conversation

hvtonev
Copy link
Contributor

@hvtonev hvtonev commented Dec 28, 2023

When TLS Mutual Authentication is removed outside of terraform the following error occurs.

╷
│ Error: 404 - Not Found:
│ 
│     Title:  Not found
│     Detail: The mutual_authentication you requested was not found
│ 
│ 
╵

Expected is to be removed from the state instead, similar to TLS Subscription

Integralist
Integralist reviewed Dec 29, 2023
View reviewed changes
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

👋🏻 Thanks for opening this PR!

I don't think the if condition works as expected. Take a look at this reduced test case and let me know your thoughts.

Thanks.

@hvtonev
Copy link
Contributor Author

hvtonev commented Dec 29, 2023 •
edited

👋🏻 Thanks for opening this PR!

I don't think the if condition works as expected. Take a look at this reduced test case and let me know your thoughts.

Thanks.

Agree. If the type assertion fails err will become nil. Thanks for pointing this out

Integralist
Integralist approved these changes Jan 2, 2024
View reviewed changes
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

Thanks for updating the PR @hvtonev

This is now approved and I'll look to get it merged shortly.

@Integralist
Copy link
Collaborator

/ok-to-test

@Integralist
ci: fix ok-to-test (fastly#796)
e136f7d 
* ci: fix ok-to-test

* ci: add missing configuration for ok-to-test behaviour
@Integralist
Copy link
Collaborator

@hvtonev would it be possible to merge the latest main into your fork's PR branch and push back up as I'm trying to validate a new CI workflow that validates the exposed interface but looks like this PR is stuck seeing the old CI job. Thanks!

@hvtonev
Copy link
Contributor Author

hvtonev commented Jan 2, 2024

@Integralist Done

@Integralist
Copy link
Collaborator

/ok-to-test

@Integralist
ci: fix ok-to-test (fastly#796)
e0f27f6 
* ci: fix ok-to-test

* ci: add missing configuration for ok-to-test behaviour
@Integralist
Copy link
Collaborator

/ok-to-test

@Integralist
ci: fix ok-to-test (fastly#796)
793c627 
* ci: fix ok-to-test

* ci: add missing configuration for ok-to-test behaviour
@Integralist
Copy link
Collaborator

/ok-to-test

@Integralist
Copy link
Collaborator

/ok-to-test sha=1fd5766

@Integralist Integralist merged commit 035a87b into fastly:main Jan 3, 2024
9 checks passed
@Integralist Integralist added the bug label Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Integralist Integralist Integralist approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@hvtonev @Integralist