Describe the bug
Version 3.1.5 introduces a breaking change: profiles with periods in them are no longer accepted when label is not specified
This commit is likely the cause: https://github.com/spring-cloud/spring-cloud-config/commit/10546f52ab5ae9a4c7017d57dce8a5ec509cfc32
After this, the defaultLabel() endpoint is no longer hit, and 404 is returned
Sample This can be replicated by making any call with a period in the profile and not supplying a label, for instance: http://localhost:8100/project/my_profile.com
Comment From: ryanjbaxter
fyi @woshikid
Comment From: woshikid
Yes, that commit caused this problem. But my_profile.com is more like a file name. The regex [^\\.]* excluded profiles with file extensions because we didn't expect a . in the profiles. Although we can rollback this commit to fix this issue, but I think it's risky to use . in profile names. Many logics depends on code like String.indexOf(), that will cause unexpected problems.
Comment From: woshikid
An example which maybe rare:
As we already support - in profiles, if we support . too, then the url http://localhost:8080/foo/bar-ext.yml will have two meanings. It matches both http://localhost:8080/{applicationName}/{profile} and http://localhost:8080/{label}/{applicationName}-{profile}.yml patterns.
- application=
foo, profile=bar-ext.yml, label=master(use default) - label=
foo, application=bar, profile=ext
And as I mentioned, there's a lot of operates on Strings in the code, which may cause unexcepted problems. I suggest not to use . in profiles.
Comment From: woshikid
Oh... I almost forgot, we also support application name with dashes, that make things more complicated.
Comment From: bencelsi-okta
Thanks for the detailed info @woshikid ! Since this is breaking backwards compatibility for certain behavior however, could this change be reverted in version 3, and maybe wait until in version 4? It is now blocking us from any future updates. We currently use many profiles with . in it, so it would also be nice to continue support of that if possible. Thanks!
Comment From: woshikid
The @GetMapping regex expression used to exclude profile names with -. After some fixes we can now support - but unfortunately the . is no longer supported. I understand your situation, but in my opinion, it's easier and safer to change the profile names than update the dependences. Because some bug fixes in these releases also causing incompatibilities. Another reason is that it is difficult to support both - and . in profile names. As I mentioned above, if this change if reverted, some urls or file paths can cause ambiguity. So, at this moment, we have to find some workaround instead of rolling back.
Comment From: josebarrueta-okta
@woshikid thank you very much for the responses to this ticket, I worked with @bencelsi-okta and our team is in charge of the CCS deployment at Okta. I understand that CCS needs to make a compromise on what it can support, at our team we have been hit with both breaking issues. Once we updated to a version that remove the support for dashes in the name - so to be able to take the version we made sure no dashes are set in our profile names. Now with the latest update we found out that . dots are not longer supported, we had to revert to upgrade to the latest library due to this issue, our worry is if this version is maintained and gets a security update of any sort we won't be able to get it and instead we will have to fork since the number of profiles to update to be compliant with the new version is massive and affects many teams.
The ask would be if possible to remove this breaking change from this version and instead bring it in a major version (4.x), we will plan the 4.x upgrade with this in mind but in the meantime we will continue be able to leverage 3.1.x updates.
Let us know if this is something that can be done. Thank you again!
Comment From: ryanjbaxter
I am going to revert the breaking change. . are valid characters in profile names. I understand that profile names with both - and . will cause some ambiguity in the controller, but that is going to be more rare than containing either a . or a -.
Comment From: woshikid
I think it's OK to revert the change. Now we need to be more careful dealing with profile names. BTW, do . also valid in application names?
Comment From: ryanjbaxter
Yes . appears to be supported in application names as well
Comment From: bencelsi-okta
Thank you!