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.

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

KAFKA-2825, KAFKA-2851: Controller failover tests added to ducktape replication tests and fix to temp dir #570

Closed
wants to merge 12 commits into from

Conversation

apovzner
Copy link
Contributor

I closed an original pull request that contained previous comments by Geoff (which are already addressed here), because I got into bad rebase situation. So, I created a new branch and cherry-picked my commits + merged with Ben's changes to fix MiniKDC tests to run on Virtual Box. That change was conflicting with my changes, where I was copying MiniKDC files with new scp method, and temp file was created inside that method. To merge Ben's changes, I added two optional parameters to scp(): 'pattern' and 'subst' to optionally substitute string while spp'ing files, which is needed for krb5.conf file.

@apovzner apovzner changed the title KAFKA-2825, KAFKA-2852: Controller failover tests added to ducktape replication tests and fix to temp dir KAFKA-2825, KAFKA-2851: Controller failover tests added to ducktape replication tests and fix to temp dir Nov 20, 2015
@@ -110,7 +111,7 @@ def __init__(self, security_protocol, interbroker_security_protocol=None, sasl_m
def client_config(self, template_props=""):
return SecurityConfig(self.security_protocol, sasl_mechanism=self.sasl_mechanism, template_props=template_props)

def setup_node(self, node):
def setup_node(self, node, miniKdc=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to set self.kdc in the constructor? And then having non-None self.kdc would be part of the logic in has_sasl_kerberos

@granders
Copy link
Contributor

granders commented Dec 1, 2015

@apovzner A few minor comments, and needs rebasing, but this looks good.

@@ -127,8 +128,9 @@ def setup_node(self, node):
jaas_conf = self.render(jaas_conf_file, node=node, is_ibm_jdk=is_ibm_jdk)
node.account.create_file(SecurityConfig.JAAS_CONF_PATH, jaas_conf)
if self.has_sasl_kerberos:
node.account.scp_to(MiniKdc.LOCAL_KEYTAB_FILE, SecurityConfig.KEYTAB_PATH)
node.account.scp_to(MiniKdc.LOCAL_KRB5CONF_FILE, SecurityConfig.KRB5CONF_PATH)
scp(miniKdc.nodes[0], MiniKdc.KEYTAB_FILE, node, SecurityConfig.KEYTAB_PATH)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just check that minikdc is not None here

@apovzner
Copy link
Contributor Author

apovzner commented Dec 1, 2015

@granders I just pushed the changes to address your comments.

@ijuma
Copy link
Contributor

ijuma commented Jan 6, 2016

Conflicts need resolving on this again unfortunately.

@ijuma
Copy link
Contributor

ijuma commented Apr 25, 2016

@apovzner, it looks like this was merged via different PRs, would you mind closing this one please?

@apovzner apovzner closed this Apr 25, 2016
@apovzner
Copy link
Contributor Author

Closed because the code in this PR was two KAFKA JIRAs so I separated it into two separate PRs and already merged.

jeffkbkim pushed a commit to jeffkbkim/kafka that referenced this pull request Oct 22, 2021
* Sync Jersey version from ccs-kafka 2.8

This is to resolve CVE in Jersey.

* Add more fixes from  KC-1603
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants