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

Move domain_certfile to the 'general' section #3512

Merged
merged 3 commits into from
Jan 25, 2022
Merged

Move domain_certfile to the 'general' section #3512

merged 3 commits into from
Jan 25, 2022

Conversation

chrzaszcz
Copy link
Member

@chrzaszcz chrzaszcz commented Jan 25, 2022 •
edited
Loading

Motivation:

  • This option affects s2s and c2s.
  • All s2s options will be placed in a map and we want domain_certfile outside of it.

Notes
This option could be changed so that each certfile is specified in the relevant host_config section, but:

  • There should be no global value - extra logic would be needed for this.
  • It would be a bit more difficult migration.
  • All certificate files are in one place, which looks tidy.

This is why I decided to keep it as a map.

The domain can be a host type, but only for c2s, so I decided to keep the option naming unchanged for now.

@codecov
Copy link

codecov bot commented Jan 25, 2022 •
edited
Loading

Codecov Report

Merging #3512 (4bd393a) into master (fd9e1d2) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3512   +/-   ##
=======================================
  Coverage   81.02%   81.02%           
=======================================
  Files         419      419           
  Lines       32309    32309           
=======================================
  Hits        26177    26177           
  Misses       6132     6132
Impacted Files Coverage Δ
src/config/mongoose_config_spec.erl 99.19% <100.00%> (ø)
src/http_upload/mod_http_upload.erl 94.62% <0.00%> (-2.16%) ⬇️
src/logger/mongoose_log_filter.erl 78.08% <0.00%> (-1.37%) ⬇️
src/pubsub/mod_pubsub_db_mnesia.erl 92.43% <0.00%> (-0.43%) ⬇️
src/mod_roster.erl 78.52% <0.00%> (-0.24%) ⬇️
src/mod_muc_log.erl 78.11% <0.00%> (ø)
src/pubsub/mod_pubsub.erl 73.31% <0.00%> (+0.11%) ⬆️
src/mod_muc_room.erl 77.26% <0.00%> (+0.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd9e1d2...4bd393a. Read the comment docs.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Jan 25, 2022 •
edited
Loading

small_tests_23 / small_tests / 6626ebf
Reports root / small

small_tests_24 / small_tests / 6626ebf
Reports root / small

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 6626ebf
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 6626ebf
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0

dynamic_domains_mysql_redis_24 / mysql_redis / 6626ebf
Reports root/ big
OK: 2674 / Failed: 0 / User-skipped: 255 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 6626ebf
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 6626ebf
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 6626ebf
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 6626ebf
Reports root/ big
OK: 1541 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / 6626ebf
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 6626ebf
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 6626ebf
Reports root/ big
OK: 3073 / Failed: 0 / User-skipped: 252 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 6626ebf
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 6626ebf
Reports root/ big
OK: 1834 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 6626ebf
Reports root/ big
OK: 1680 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0

@chrzaszcz chrzaszcz force-pushed the domain-certfile-general branch 2 times, most recently from ecc7197 to 0a3897a Compare January 25, 2022 08:24
@mongoose-im

This comment has been minimized.

Sign in to view
@mongoose-im
Copy link
Collaborator

mongoose-im commented Jan 25, 2022 •
edited
Loading

small_tests_23 / small_tests / 0a3897a
Reports root / small

small_tests_24 / small_tests / 0a3897a
Reports root / small

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 0a3897a
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 0a3897a
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0

dynamic_domains_mysql_redis_24 / mysql_redis / 0a3897a
Reports root/ big
OK: 2674 / Failed: 0 / User-skipped: 255 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 0a3897a
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 0a3897a
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 0a3897a
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 0a3897a
Reports root/ big
OK: 1541 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / 0a3897a
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 0a3897a
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 0a3897a
Reports root/ big
OK: 1834 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 0a3897a
Reports root/ big
OK: 3073 / Failed: 0 / User-skipped: 252 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 0a3897a
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 0a3897a
Reports root/ big
OK: 1680 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0

@chrzaszcz chrzaszcz marked this pull request as ready for review January 25, 2022 08:43
@chrzaszcz
Move domain_certfile to the 'general' section
43e1743 
Motivation:
- This option affects s2s and c2s
- All s2s options will be placed in a map

Also: check if the files exist
@chrzaszcz
Update tests after moving domain_certfile
e844cc0 
Certificate files need to exist now
@chrzaszcz chrzaszcz force-pushed the domain-certfile-general branch from 0a3897a to 302b41f Compare January 25, 2022 08:53
@mongoose-im
Copy link
Collaborator

mongoose-im commented Jan 25, 2022 •
edited
Loading

small_tests_24 / small_tests / 302b41f
Reports root / small

small_tests_23 / small_tests / 302b41f
Reports root / small

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 302b41f
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0

dynamic_domains_mysql_redis_24 / mysql_redis / 302b41f
Reports root/ big
OK: 2674 / Failed: 0 / User-skipped: 255 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 302b41f
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 302b41f
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 302b41f
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 302b41f
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

pgsql_mnesia_23 / pgsql_mnesia / 302b41f
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 302b41f
Reports root/ big
OK: 1541 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 302b41f
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 302b41f
Reports root/ big
OK: 1834 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 302b41f
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 302b41f
Reports root/ big
OK: 3073 / Failed: 0 / User-skipped: 252 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 302b41f
Reports root/ big
OK: 1680 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0

@chrzaszcz chrzaszcz force-pushed the domain-certfile-general branch from 302b41f to 4bd393a Compare January 25, 2022 09:29
@mongoose-im
Copy link
Collaborator

mongoose-im commented Jan 25, 2022 •
edited
Loading

small_tests_24 / small_tests / 4bd393a
Reports root / small

small_tests_23 / small_tests / 4bd393a
Reports root / small

dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 4bd393a
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 4bd393a
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0

dynamic_domains_mysql_redis_24 / mysql_redis / 4bd393a
Reports root/ big
OK: 2674 / Failed: 0 / User-skipped: 255 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 4bd393a
Reports root/ big
OK: 2691 / Failed: 0 / User-skipped: 238 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 4bd393a
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

ldap_mnesia_23 / ldap_mnesia / 4bd393a
Reports root/ big
OK: 1500 / Failed: 0 / User-skipped: 390 / Auto-skipped: 0

internal_mnesia_24 / internal_mnesia / 4bd393a
Reports root/ big
OK: 1541 / Failed: 0 / User-skipped: 349 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 4bd393a
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 4bd393a
Reports root/ big
OK: 3077 / Failed: 1 / User-skipped: 247 / Auto-skipped: 0

mam_SUITE:rdbms_async_pool_mam_all:archived:metrics_incremented_for_async_pools 
{error,{test_case_failed,"ASSERT EQUAL\n\tExpected false\n\tValue true\n"}}

Report log

pgsql_mnesia_23 / pgsql_mnesia / 4bd393a
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0

elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 4bd393a
Reports root/ big
OK: 1834 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0

mysql_redis_24 / mysql_redis / 4bd393a
Reports root/ big
OK: 3073 / Failed: 0 / User-skipped: 252 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 4bd393a
Reports root/ big
OK: 1680 / Failed: 0 / User-skipped: 363 / Auto-skipped: 0

mssql_mnesia_24 / odbc_mssql_mnesia / 4bd393a
Reports root/ big
OK: 3078 / Failed: 0 / User-skipped: 247 / Auto-skipped: 0

@mongoose-im mongoose-im mentioned this pull request Jan 25, 2022
Closed
gustawlippa
gustawlippa approved these changes Jan 25, 2022
View reviewed changes
Copy link
Contributor

@gustawlippa gustawlippa left a comment

Choose a reason for hiding this comment

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

Looks good, the option does much more sense in the general section.

@gustawlippa gustawlippa merged commit a0e9c6a into master Jan 25, 2022
@gustawlippa gustawlippa deleted the domain-certfile-general branch January 25, 2022 10:59
@Premwoik Premwoik added this to the 5.1.0 milestone May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gustawlippa gustawlippa gustawlippa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
5.1.0
Development

Successfully merging this pull request may close these issues.
4 participants
@chrzaszcz @mongoose-im @gustawlippa @Premwoik