Skip to content

MINOR: Rejoin split ssl principal mapping rules#6099

Merged
omkreddy merged 4 commits intoapache:trunkfrom
acres4:MINOR-rejoin_split_ssl_principal_mapping_rules
Jan 21, 2019
Merged

MINOR: Rejoin split ssl principal mapping rules#6099
omkreddy merged 4 commits intoapache:trunkfrom
acres4:MINOR-rejoin_split_ssl_principal_mapping_rules

Conversation

@ryannatesmith
Copy link
Copy Markdown
Contributor

@ryannatesmith ryannatesmith commented Jan 7, 2019

The string in the properties file is split on commas such that ssl.principal.mapping.rules=RULE:^CN=(.*?),OU=ServiceUsers.*$/$1/ will create an array of ["RULE:^CN=(.*?)", "OU=ServiceUsers.*$/$1/"]. The Kafka broker then throws an illegal argument exception on startup because these rules are invalid. This pull request merges strings back together such that all rules either begin with RULE: or DEFAULT.

The new code was tested by adding two new functions to SslPrincipalMapperTest that mimic the existing tests, but with string lists as they would be read from the properties file as mentioned above.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

ryannatesmith added 2 commits January 7, 2019 11:45
Java properties splits the configuration array on commas, and that leads to rules containing commas being split before being evaluated. This commit adds a code change to re-join those strings into full rules before evaluating them. The function assumes every rule is either DEFAULT or begins with the prefix RULE:
@omkreddy
Copy link
Copy Markdown
Contributor

omkreddy commented Jan 8, 2019

@ryannatesmith Thanks for catching this! LGTM.

@omkreddy
Copy link
Copy Markdown
Contributor

@ryannatesmith left couple of minor code changes

omkreddy and others added 2 commits January 14, 2019 09:06
…PrincipalMapper.java

Co-Authored-By: ryannatesmith <natesmith@yandex.ru>
…PrincipalMapper.java

Co-Authored-By: ryannatesmith <natesmith@yandex.ru>
@omkreddy omkreddy changed the title Minor rejoin split ssl principal mapping rules MINOR: Rejoin split ssl principal mapping rules Jan 15, 2019
Copy link
Copy Markdown
Contributor

@omkreddy omkreddy left a comment

Choose a reason for hiding this comment

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

@ryannatesmith Thanks for the PR. LGTM.

@ryannatesmith
Copy link
Copy Markdown
Contributor Author

@rajinisivaram can you take a look at this pull request? It is related to the work in KAFKA-5462 and pull request 5684

@omkreddy
Copy link
Copy Markdown
Contributor

Merging to trunk

@omkreddy omkreddy merged commit e75e473 into apache:trunk Jan 21, 2019
abbccdda pushed a commit to abbccdda/kafka that referenced this pull request Jan 24, 2019
* Join ssl principal mapping rules correctly before evaluating.

Java properties splits the configuration array on commas, and that leads to rules containing commas being split before being evaluated. This commit adds a code change to re-join those strings into full rules before evaluating them. The function assumes every rule is either DEFAULT or begins with the prefix RULE:
jarekr pushed a commit to confluentinc/kafka that referenced this pull request Apr 18, 2019
* ak/trunk:
  MINOR: fix race condition in KafkaStreamsTest (apache#6185)
  KAFKA-4850: Enable bloomfilters (apache#6012)
  MINOR: ducker-ak: add down -f, avoid using a terminal in ducker test
  KAFKA-5117: Stop resolving externalized configs in Connect REST API
  MINOR: Cleanup handling of mixed transactional/idempotent records (apache#6172)
  KAFKA-7844: Use regular subproject for generator to fix *All targets (apache#6182)
  Fix Documentation for cleanup.policy is out of date (apache#6181)
  MINOR: increase timeouts for KafkaStreamsTest (apache#6178)
  MINOR: Rejoin split ssl principal mapping rules (apache#6099)
  MINOR: Handle case where connector status endpoints returns 404 (apache#6176)
  MINOR: Remove unused imports, exceptions, and values (apache#6117)
  KAFKA-3522: Add internal RecordConverter interface (apache#6150)
  Fix Javadoc of KafkaConsumer (apache#6155)
  KAFKA-6455: Extend CacheFlushListener to forward timestamp (apache#6147)
  MINOR: Log partition info when creating new request batch in controller (apache#6145)
  KAFKA-7652: Part I; Fix SessionStore's findSession(single-key) (apache#6134)
  MINOR: Remove the InvalidTopicException handling in InternalTopicManager (apache#6167)
  [KAFKA-7024] Rocksdb state directory should be created before opening the DB (apache#6138)
  MINOR:: Fix typos (apache#6079)
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
* Join ssl principal mapping rules correctly before evaluating.

Java properties splits the configuration array on commas, and that leads to rules containing commas being split before being evaluated. This commit adds a code change to re-join those strings into full rules before evaluating them. The function assumes every rule is either DEFAULT or begins with the prefix RULE:
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