Skip to content

Conversation

@joshk
Copy link
Contributor

@joshk joshk commented Feb 16, 2025

The certs_keys config key was added recently(ish) to the Erlang SSL stack.

https://www.erlang.org/doc/apps/ssl/ssl.html#t:cert_key_conf/0

The `certs_keys` config key was added recently(ish) to the Erlang SSL stack.

https://www.erlang.org/doc/apps/ssl/ssl.html#t:cert_key_conf/0
@josevalim
Copy link
Member

Thanks! Could we perhaps have a unit test for it?

@joshk
Copy link
Contributor Author

joshk commented Feb 16, 2025

Thanks! Could we perhaps have a unit test for it?

Sure, I'll work on that tomorrow. I couldn't find any tests for the keyfile and certfile logic, so didn't know if tests would be required for this.

How do you suggest I handle the OTP app logic?

  defp otp_app(options) do
    if app = options[:otp_app] do
      Application.app_dir(app)
    else
      fail("the :otp_app option is required when setting relative SSL certfiles")
    end
  end

Co-authored-by: José Valim <jose.valim@gmail.com>
@josevalim
Copy link
Member

Perhaps you could use :plug as the otp app itself and, if necessary, have it pick files from tmp? To be clear, it doesn't need to be an integration test, testing the configure function is enough. :)

@joshk
Copy link
Contributor Author

joshk commented Feb 16, 2025

Funny timing, I just thought about using plug as the app.

Cool cool, that was my plan. I'll just test configure, which validates and throws an error if the OTP app can't be found.

I'll have this done in the morning. I'm working on a nicer Acme integration to use for NervesHub and NervesCloud, and this is part of the puzzle.

@joshk
Copy link
Contributor Author

joshk commented Feb 16, 2025

@josevalim done! although I did need to essentially revert your commit suggestion due to it not working as expected.

@joshk joshk requested a review from josevalim February 16, 2025 22:18
@josevalim josevalim merged commit 187266d into elixir-plug:main Feb 17, 2025
2 checks passed
@joshk
Copy link
Contributor Author

joshk commented Feb 17, 2025

@josevalim thank you for the merge! its almost like the good ol' days when you were mentoring me with Rails contributions :D

Do you have any plans for a new release?

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