Skip to content

Deprecate disableSSLRedirect with sslRedirect - #105

Open
nilathedragon wants to merge 1 commit into
caddyserver:mainfrom
Infinytum:feat/rename-ssl-redirect
Open

nilathedragon wants to merge 1 commit into
caddyserver:mainfrom
Infinytum:feat/rename-ssl-redirect

Conversation

@nilathedragon

Copy link
Copy Markdown
Contributor

For issue #102

Should we log a deprecation notice if we detect ingress objects using the old annotation?
Could be done easily in matcher.go if we add a zap logger to the plugin.

@Embraser01

Copy link
Copy Markdown
Member

Should we log a deprecation notice if we detect ingress objects using the old annotation?
Could be done easily in matcher.go if we add a zap logger to the plugin.

Fine by me

@nilathedragon

Copy link
Copy Markdown
Contributor Author

image

Hmm putting it there spams the logs quite a bit. It would certainly bring the message across 😅

@Embraser01

Copy link
Copy Markdown
Member

image

Hmm putting it there spams the logs quite a bit. It would certainly bring the message across 😅

It may be too much spam 😅, the convert function may be called many times so I think we should log this warning in another way, like only if config changed. That would mean having a list of deprecation messages to log somewhere 🤔

I'm not sure on how to do that...

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants