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

[pulsar-broker] validate namespace isolation policy regex before updating #8804

Merged
merged 1 commit into from
Dec 23, 2020

Conversation

rdhabalia
Copy link
Contributor

Motivation

Right now, the broker stores ns-isolation policy regex without validating which will cause failure at runtime. So, perform fast fail validation and validate policy before storing instead of failing at runtime with invalid policy regex.

23:14:56.710 [pulsar-1-11] WARN  o.a.p.b.l.i.SimpleResourceAllocationPolicies - isPrimaryForAnyNamespace: [{}]
java.util.regex.PatternSyntaxException: Illegal character range near index 16
test-broker1[45-46].uswest.com.*
                 ^
        at java.util.regex.Pattern.error(Pattern.java:1955) ~[na:1.8.0_131]
        at java.util.regex.Pattern.range(Pattern.java:2655) ~[na:1.8.0_131]
        at java.util.regex.Pattern.clazz(Pattern.java:2562) ~[na:1.8.0_131]
        at java.util.regex.Pattern.sequence(Pattern.java:2063) ~[na:1.8.0_131]
        at java.util.regex.Pattern.expr(Pattern.java:1996) ~[na:1.8.0_131]
        at java.util.regex.Pattern.compile(Pattern.java:1696) ~[na:1.8.0_131]
        at java.util.regex.Pattern.<init>(Pattern.java:1351) ~[na:1.8.0_131]
        at java.util.regex.Pattern.compile(Pattern.java:1028) ~[na:1.8.0_131]
        at java.util.regex.Pattern.matches(Pattern.java:1133) ~[na:1.8.0_131]
        at java.lang.String.matches(String.java:2121) ~[na:1.8.0_131]
        at org.apache.pulsar.common.policies.impl.NamespaceIsolationPolicyImpl.matchesBrokerRegex(NamespaceIsolationPolicyImpl.java:104)

@rdhabalia rdhabalia added this to the 2.8.0 milestone Dec 3, 2020
@rdhabalia rdhabalia self-assigned this Dec 3, 2020
@rdhabalia
Copy link
Contributor Author

/pulsarbot run-failure-checks

@rdhabalia
Copy link
Contributor Author

/pulsarbot run-failure-checks

@zymap
Copy link
Member

zymap commented Dec 8, 2020

The test is failed by this.

Error:  testBookKeeperDown(org.apache.pulsar.tests.integration.cli.HealthCheckTest)  Time elapsed: 0.194 s  <<< FAILURE!
org.apache.pulsar.tests.integration.docker.ContainerExecException: pkill -STOP -f BookieServer failed on e3f20778f4944a0efbec3d50a936bc48e8d43e87f0bfe514dd6e80be937546ec with error code 1
	at org.apache.pulsar.tests.integration.utils.DockerUtils$2.onComplete(DockerUtils.java:259)
	at com.github.dockerjava.core.exec.AbstrAsyncDockerCmdExec$1.onComplete(AbstrAsyncDockerCmdExec.java:51)
	at com.github.dockerjava.okhttp.OkHttpInvocationBuilder.lambda$executeAndStream$4(OkHttpInvocationBuilder.java:331)
	at java.lang.Thread.run(Thread.java:748)

I remember it already fixed in PR #8791. So could you please rebase to the latest master?

…ting

[pulsar-broker] validate namespace isolation policy regex before updating
@rdhabalia rdhabalia reopened this Dec 22, 2020
@rdhabalia
Copy link
Contributor Author

/pulsarbot run-failure-checks

@rdhabalia rdhabalia merged commit ab9c77a into apache:master Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants