Skip to content

Reject AppCookie LB stickiness policy on Virtual Router networks #13315

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

Open
sureshanaparti wants to merge 5 commits into
base: 4.20
Choose a base branch
from
Open

Reject AppCookie LB stickiness policy on Virtual Router networks #13315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9111855
Deprecate AppCookie (App session) based stickiness type policy for Vi…
sureshanaparti Jun 1, 2026
5bdd22a
some code improvements
sureshanaparti Jun 2, 2026
7404a52
Revert "some code improvements"
sureshanaparti Oct 1, 2026
46864a0
Revert "Deprecate AppCookie (App session) based stickiness type polic…
sureshanaparti Oct 1, 2026
e91b9e5
Reject AppCookie LB stickiness policy on Virtual Router networks
sureshanaparti Oct 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 2 additions & 20 deletions server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -945,26 +945,8 @@ public boolean validateHAProxyLBRule(final LoadBalancingRule rule) {

}
} else if (LbStickinessMethod.StickinessMethodType.AppCookieBased.getName().equalsIgnoreCase(stickinessPolicy.getMethodName())) {
String length = null; // optional
String holdTime = null; // optional

for (final Pair<String, String> paramKV : paramsList) {
final String key = paramKV.first();
final String value = paramKV.second();
if ("length".equalsIgnoreCase(key)) {
length = value;
}
if ("holdtime".equalsIgnoreCase(key)) {
holdTime = value;
}
}

if (length != null && !containsOnlyNumbers(length, null)) {
throw new InvalidParameterValueException(String.format("Failed LB in validation rule id: %s Cause: length is not a number: %s", rule.getLb(), length));
}
if (holdTime != null && !containsOnlyNumbers(holdTime, timeEndChar) && !containsOnlyNumbers(holdTime, null)) {
throw new InvalidParameterValueException(String.format("Failed LB in validation rule id: %s Cause: holdtime is not in timeformat: %s", rule.getLb(), holdTime));
}
throw new InvalidParameterValueException(String.format("Failed LB in validation rule: %s Cause: AppCookie based stickiness "

@Damans227 Damans227 Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what happens to lb rules that already have an app cookie policy? looks like editing them will now fail until someone removes the policy

@weizhouapache weizhouapache Oct 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

appcookie has been deprecated in haproxy from several versions ago

@Damans227 Damans227 Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

makes sense, those rules already break the router config today so a clear error on edit is better

+ "(HAProxy 'appsession') is not supported on a Virtual Router, as this directive was removed in HAProxy 1.6", rule.getLb()));

@Damans227 Damans227 Oct 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should we also drop app cookie from the list of methods the router says it supports? right now the ui still offers it and then it fails here

}
}
return true;
Expand Down
46 changes: 46 additions & 0 deletions server/src/test/java/com/cloud/network/router/NetworkHelperImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,15 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.cloud.configuration.Config;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.network.lb.LoadBalancingRule;
import com.cloud.network.rules.LbStickinessMethod;
import com.cloud.network.rules.LoadBalancer;
import com.cloud.utils.Pair;
import com.cloud.utils.net.Ip;
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.network.router.deployment.RouterDeploymentDefinition;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -57,6 +65,10 @@
import com.cloud.vm.dao.DomainRouterDao;
import com.cloud.vm.dao.NicDao;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;


@RunWith(MockitoJUnitRunner.class)
public class NetworkHelperImplTest {
Expand Down Expand Up @@ -91,10 +103,15 @@ public class NetworkHelperImplTest {
@Mock
private VMTemplateVO template;

@Mock
private ConfigurationDao configDao;

@Before
public void setUp() {
nwHelper._networkDao = networkDao;
nwHelper._networkModel = networkModel;
nwHelper._configDao = configDao;
lenient().when(configDao.getValue(Config.NetworkLBHaproxyStatsPort.key())).thenReturn("8081");
when(template.getId()).thenReturn(1L);
when(template.isDynamicallyScalable()).thenReturn(true);
when(virtualProvider.getId()).thenReturn(1L);
Expand Down Expand Up @@ -268,4 +285,33 @@ public void testUpdateDomainRouter() {
assertEquals(Hypervisor.HypervisorType.KVM, result.getHypervisorType());
assertTrue(result.isDynamicallyScalable());
}

@Test(expected = InvalidParameterValueException.class)
public void testValidateHAProxyLBRuleRejectsAppCookieStickiness() {
LoadBalancer lb = mock(LoadBalancer.class);
when(lb.getSourcePortStart()).thenReturn(80);

List<Pair<String, String>> params = new ArrayList<>();
params.add(new Pair<>("cookie-name", "JSESSIONID"));
LoadBalancingRule.LbStickinessPolicy stickinessPolicy =
new LoadBalancingRule.LbStickinessPolicy(LbStickinessMethod.StickinessMethodType.AppCookieBased.getName(), params);
LoadBalancingRule rule = new LoadBalancingRule(lb, new ArrayList<>(), Collections.singletonList(stickinessPolicy), new ArrayList<>(), mock(Ip.class));

nwHelper.validateHAProxyLBRule(rule);
}

@Test
public void testValidateHAProxyLBRuleAllowsSourceBasedStickiness() {
LoadBalancer lb = mock(LoadBalancer.class);
when(lb.getSourcePortStart()).thenReturn(80);

List<Pair<String, String>> params = new ArrayList<>();
params.add(new Pair<>("tablesize", "200k"));
params.add(new Pair<>("expire", "30m"));
LoadBalancingRule.LbStickinessPolicy stickinessPolicy =
new LoadBalancingRule.LbStickinessPolicy(LbStickinessMethod.StickinessMethodType.SourceBased.getName(), params);
LoadBalancingRule rule = new LoadBalancingRule(lb, new ArrayList<>(), Collections.singletonList(stickinessPolicy), new ArrayList<>(), mock(Ip.class));

assertTrue(nwHelper.validateHAProxyLBRule(rule));
}
}
Loading