Skip to content

Conversation

@ca61688
Copy link
Collaborator

@ca61688 ca61688 commented Jan 29, 2026

Make sure you have checked all steps below.

Issue

Tests

  • My PR adds the following tests based on our test strategy OR does not need testing for this extremely good reason:
    • MyUnitTest

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it, or I have linked to a
    separate issue for that below.
  • If I have added new Java code, I have added Javadoc that explains it following our conventions and style.
  • If I have added or removed any dependencies from the project, I have updated the NOTICES file.

@ca61688 ca61688 linked an issue Jan 29, 2026 that may be closed by this pull request
@ca61688 ca61688 marked this pull request as ready for review January 30, 2026 11:23
@ca61688 ca61688 added the needs-reviewer Pull requests that need a reviewer to be assigned label Jan 30, 2026
if (split.length % 2 != 0) { //Check for even number of tag name and values
throw new IllegalArgumentException("Tags element didn't contain an even number of elements." +
" Ensure each tag name and value are seperated by a comma.");
}
Copy link
Collaborator

@patchwork01 patchwork01 Jan 30, 2026

Choose a reason for hiding this comment

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

The admin client will still crash when this is thrown.

If the tags property value is invalid, it needs to fail during validation of the properties. Right now this failure is thrown while creating the InstanceProperties object, before we can call validate on the object.

When this is done through the admin client, we expect the validation failure to be caught in UpdatePropertiesRequest.getInvalidProperties. That finds out exactly which properties are invalid, and that's then displayed to the user in PropertiesDiff.print.

We need to provide unit test coverage for this somehow. I think the simplest approach would be to cover it in InstancePropertiesTest. We can call InstanceProperties.createWithoutValidation, with an invalid value for the tags property. We can assert that an exception is only thrown when you call InstanceProperties.validate. We can also assert that it should fail when you try to read the tags when they're invalid.

Sorry the information on the issue is a bit vague. I'll add more information there as well.

@patchwork01 patchwork01 removed the needs-reviewer Pull requests that need a reviewer to be assigned label Jan 30, 2026
@patchwork01 patchwork01 marked this pull request as draft January 30, 2026 13:51
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.

Adminclient.sh crash when sleeper.tags property is malformed

3 participants