fix: clear role members when set to trust role

[TakuyaMatsu]

Description

When creating or editing a Role with the Trust field set via SolutionTemplate, a role is created that has both Trust and role members. We confirmed the problem mainly in the following two patterns:

• Adding a SolutionTemplate that sets both RoleMember and Trust to the server and using it.
• Applying a SolutionTemplate that sets Trust to an already-created role that already has members.

This change fixes the issue so that it does not occur in the patterns described above.

Contribution Checklist:

• The pull request does not introduce any breaking changes
• I have read the contribution guidelines.
• Create an issue and link to the pull request.

Attach Screenshots (Optional)

[gemini-code-assist]

Summary of Changes

Hello @TakuyaMatsu, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug in the SolutionTemplate functionality where roles configured with a Trust relationship were not correctly clearing their roleMembers. The change ensures that when a SolutionTemplate is used to define or update a role with a Trust attribute, any explicit or existing roleMembers are removed, aligning the role's configuration with the intended trust-based access model. This prevents unintended access and maintains the integrity of role definitions.

Highlights

• Role Member Clearing: When a role is created or updated via a SolutionTemplate and configured with a Trust relationship, any associated roleMembers are now explicitly cleared.
• Addressing Specific Scenarios: This fix resolves issues where roles would incorrectly retain members when a SolutionTemplate defined both RoleMember and Trust, or when a Trust-based template was applied to an existing role with members.
• Enhanced Test Coverage: New unit tests have been added to validate that roles configured with Trust through SolutionTemplates correctly have their members cleared, covering both creation and update scenarios.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request fixes an issue where applying a solution template could result in a role having both a trust relationship and members, which is an invalid state. The changes correctly clear role members when a trust is set via a template, and ignore members defined in a template if a trust is also present. The fix is implemented in DBService.java and is well-covered by two new test cases that validate both scenarios. The changes look correct and improve the consistency of role management through templates. I've added a minor suggestion to improve test isolation in the new test cases.

[havetisyan]

I'm not sure if I follow this PR fix. It is completely invalid to specify a role with both trust and members. If you do that through the API, you should get back 400 request error.

Now, as an admin, you're forcing an incorrect state through solution template. So the server must not apply that template since it's invalid if it would have come through API.

As an admin, it's your responsibility to generate the valid template (this is why it's an admin operation and users are not allowed to create template).

The correct fix for this problem would be that the server must validate the solution template and not just assume that admin did the right thing and reject the template during startup and require the admin to fix the template to make it valid

[TakuyaMatsu]

@havetisyan

Thank you for the feedback.

I agree admins should ship valid templates. This change is meant as a guardrail to prevent human mistakes from creating a bad state.

What I’m seeing right now

If I apply a SolutionTemplate that sets trust to a role that already has members (and the template doesn’t set roleMembers), the merged role ends up with both. getRole looks fine, but /sys/modified_domains shows both fields, e.g.:

{
  "name": "home.takuya:role.hoge",
  "modified": "2025-10-08T00:18:19.170Z",
  "roleMembers": [
    { "memberName": "user.takuya", "principalType": 1 }
  ],
  "trust": "hoge"
}

That breaks the “trust XOR members” rule and confuses anything reading modified_domains.

Plan

• Validate: reject any template that defines both trust and roleMembers for the same role (same idea as the 400 the API would return).
• Merge rule: if a template sets trust on an existing role and doesn’t specify roleMembers, clear existing members so the result stays valid and the op is idempotent.
  This PR implements the merge rule.

[havetisyan]

@TakuyaMatsu your PR just masks invalid state and allows it to continue on so we shouldn't promote that.

1. If you specify a template that includes both role members and trusts - it should be rejected during startup
2. If you specify template with trust and apply it to an existing role which is already referenced in the template but has members, it should be rejected.

In both case the correct operation is to reject the invalid state. Otherwise, the merge may not be what the user wants and as such it should not be allowed

[TakuyaMatsu]

@havetisyan

1. If you specify a template that includes both role members and trusts - it should be rejected during startup

Right now we only validate JSON format at startup. Should I add logic here to stop startup if the template changes both trust and roleMembers?

athenz/servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java

Lines 1293 to 1317 in fa063a2

	void loadSolutionTemplates() {
	// get the configured path for the list of service templates
	String solutionTemplatesFname = System.getProperty(ZMSConsts.ZMS_PROP_SOLUTION_TEMPLATE_FNAME,
	getRootDir() + "/conf/zms_server/solution_templates.json");
	Path path = Paths.get(solutionTemplatesFname);
	try {
	serverSolutionTemplates = JSON.fromBytes(Files.readAllBytes(path), SolutionTemplates.class);
	} catch (IOException ex) {
	LOG.error("Unable to parse solution templates file {}: {}",
	solutionTemplatesFname, ex.getMessage());
	}
	if (serverSolutionTemplates == null) {
	LOG.error("Generating empty solution template list...");
	serverSolutionTemplates = new SolutionTemplates();
	serverSolutionTemplates.setTemplates(new HashMap<>());
	serverSolutionTemplateNames = Collections.emptyList();
	} else {
	serverSolutionTemplateNames = new ArrayList<>(serverSolutionTemplates.names());
	Collections.sort(serverSolutionTemplateNames);
	}
	}

2. If you specify template with trust and apply it to an existing role which is already referenced in the template but has members, it should be rejected.

My understanding is that SolutionTemplate is merge-oriented. In practice, when applying a template to an existing role, fields like members are merged rather than hard-replaced. If we reject applying a template that sets trust to a role that already has members, we’d be introducing an implicit rule—“you can’t set trust via SolutionTemplate on roles with existing members.” I’d prefer to avoid that.

[havetisyan]

1. Yes, after we parse the solution templates, we should go through the objects and validate that the roles don't contain both members and trust attributes.
2. Solution Templates feature is merge oriented but it's not designed to handle different types of roles. If you have a delegated role and your template is for a regular member role, there is some disconnect that should not be allowed. If your role is a delegated role and you apply a template which makes the role as member role, you can't just ignore the members and return success since that assumes according to the template it was supposed to be a member role. Similarly, if you have a member role and your template tries to make the role as delegated, again, you can't just return success since that means you think the role is now a delegated role but in reality its not.

The server should check the role when modifying and it's not the same type as the template role that it's trying to apply, it should reject the operation.

[gyakami]

@havetisyan
I'll take over this task. As you suggested, I plan to change the API to return an error. However, since this is a breaking change, I'd like to propose a phased approach.
First, I want to emit warning logs only, to prompt affected users to fix their configurations. Then, we can eventually transition to returning an error. Is this approach acceptable?
I've created PR #3159 for this. I would appreciate your review.