Hi list,
I’ve just created a GSIP (216) with a proposal to make several improvements to GeoFence.
Please see https://github.com/geoserver/geoserver/wiki/GSIP-216 for details.
camptocamp
INNOVATIVE SOLUTIONS
BY OPEN SOURCE EXPERTS
Gabriel Roldán
Geospatial Developer
+1
Looks like a good plan to me
Ian
Ian Turton
Dear Gabriel,
I went through your proposal; I will start with some comments about the motivation section:
That said, I’m quite concerned with the modules refactoring:
My proposal is to split the current proposed work in two phases:
Looking forward in reading your feedback.
Cheers,
Emanuele
[1] https://github.com/geoserver/geofence/issues/128
[2] https://github.com/geoserver/geofence/wiki/GeoFence-REST-API
Ing. Emanuele Tajariol
Technical Lead
GeoSolutions Group
mobile: +39 347 7895230
office: +39 0584 962313
fax: +39 0584 1660272
Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia.
This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail.
Hi Emmanuele,
Thanks for the proposal review and comments.
First and foremost, let me stress out the proposal is to create a branch, 4.0.x.
Since there are several topics that of course need to be discussed and agreed
upon, all this work would serve as a concrete platform to do that, with as much
proof as possible that everything keeps working.
Not to merge or force any of this in the current 3.x series.
Follow up inline.
Dear Gabriel,
I went through your proposal; I will start with some comments about the motivation section:
- The persistence abstractions are powered by Google’s Generic DAO Framework, which is not maintained since years ago
- This refactoring was already completed and working, as reported in this issue:
- JDK11 compatibility #128 [1]
Oh that’s interesting. Unfortunately I couldn’t know, or at least it was easy to oversight,
since that issue is three years old, it’s not merged, and the title does not relate to the persistence
framework; though the description does indeed mention it. Should I’ve noticed it, it would have
saved me a ton of work.
- All the GenericDAO constructs were ported to Hibernate’s criteria and to some newly created classes to reduce the impact on the existing code as much as possible.
Yes, the Hibernate criteria API also makes sense.
Personally, I prefer QueryDSL for the type-safety part. QueryDSL code-generates classes that
let you create arbitrary queries in a type safe way, and adds no runtime dependencies, just a
compile-time @Entity annotation processor.
In any case this and everything else is up to discussion.
- The REST APIs exposed by GeoFence’s standalone server application, and the GeoFence embedded engine in GeoServer, have some differences, which makes it hard to use a REST client interchangeably.
- When the REST APIs were reimplemented in the embedded configuration (in the geofence-server module), they did not follow the detailed documentation available at GeoFence REST API [2] , that was followed in the initial (standalone) implementation. I would not create a third API, but would stick to the existing one: breaking backwards compatibility is something we try very hard not to do in GeoServer, since there are deployed systems depending on it that would break on upgrade.
As much as I very much agree with the principle, we’re kind of in an impossible situation here.
I agree having two disparate REST APIs is bad enough.
On the other hand, I have been working for a year now on a system that needs to interface with GeoFence.
I would rather have the chance to deploy GeoFence standalone or embedded, and be able to use the same
REST API regardless, but that’s impossible without tons of hacks.
It’s also difficult to implement a client even if you choose one of them, since there are some inconsistencies like
requests to create a Rule accepting a given content type (e.g. application/json) but returning text/plain with the
rule id, and then expecting long to fetch the rule.
Additionally, in order to replace Spring RMI with an API client, the client would need to implement GeoFence standalone’s
REST API, but a client working against GeoServer would implement GeoFence embeeded’s REST API, so we’re just multiplying
the maintenance problem.
So my proposal is to:
1- use an API-first approach through an OpenAPI specification that’s common to both kinds of deployments.
2- Maintain the current embedded API for as long as necessary in GeoServer.
3- Also expose the new API as v2, where anyone can use the provided or generate ready to use client libraries in different programming languages
(I’ve actually just finished porting both geoserver plugins, will send a link once I get the configuration part working too in a backward compatible way)
Thinking of an upgrade path:
For GeoServer embedded, REST API wise, wouldn’t make any difference, other than v2 also being available, or not even, could be a configuration choice.
For standalone, an upgrade would also mean a standalone GeoFence upgrade, so it doesn’t matter if it uses RMI or the new OpenAPI interface to communicate with GeoFence
Regarding the other points, it’s great getting rid of the older libraries (we had to revive the old Hibernate Spatial in order to keep up with the JTS upgrades – see https://github.com/geosolutions-it/hibernate-spatial).
So, apart from the notes above, I’m good with the various motivations and points in the proposal, because they aim to get rid of older unmaintained libraries and to keep the libs aligned with the ones used in GeoServer.
nice!
That said, I’m quite concerned with the modules refactoring:
- They are not strictly needed and motivated by the previous points.
While I disagree (again, for a longer term, on 4.x) , I see my mistake in not emphasizing
on the motivation section and only mentioning sharing the maintenance burden in the
overview section, I guess I just didn’t want to come across as arrogant or patronizing.
Since we’re kicking off the discussion, allow me to elaborate:
I know both Alessio and yourself shall know the codebase from the heart, and that’s great,
but bear with me, as an outsider and newcomer to the project, I had a
rather hard time getting acquainted with it, hence my intention is to make sure the architecture
is very clear and upfront, not only for us but for any other potential collaborators. Something
closer to a Screaming Architecture, as my initial struggles were related to figuring out the architecture,
and how components relate to each other, not from the persistence viewpoint but from
the business domain view.
If you look at the following comparison of 3.x and 4.x proposed modularity, I think the 3.x one doesn’t
say much about what the software system is or does, but instead I see there are apis, persistence, and
a gui.
The 4.x proposal on the other hand makes it very clear that the business domain has 4 domain units
and a common object model, and that’s all one would need to look into to understand GeoFence.
No Spring, no JPA, no REST, no anything else than pure domain specific logic.
Everything else is integration and implementation detail you can dig into if need be.
- They will likely make it difficult to review and functionally validate.
They might. Hence all functional tests are in the domain modules themselves, without requiring digging
into integration details, while at the same time serve as base integration tests for both the persistence
and client api integrations.
The tests (e.g. the most important being RuleReaderServiceImplTest) run against in-memory
persistence abstraction implementations that use RuleFilter itself as predicate, through the
new RuleFilter.matches(Rule):boolean method, and that helped me a lot to make sure both the openapi
client and JPA backed Repositories do the right thing and are hence interchangeable as backing
Repositories for the different services.
Other things like making the object model immutable also helps to clearly reason about things, like in
avoiding side effects with argument objects being changed by functions.
- They may introduce further issues, along with the potential ones caused by the already big library upgrade shock.
Yes they would, as there’s no such thing as bug-free software is it? On the bright side we will be taking on the burden of stress
testing and fixing for our current customer, and another one in the very near future.
- They may be a blocker for the current maintainers to keep up the work and to respond quickly to the issues that will pop up, especially in the first times it will be released.
Indeed. So once again, no rush. I’m only asking for a branch so we have a common place to review, discuss, and
coordinate on things. I honestly think having this starting point is much better than trying to make one proposal at a time
because it’s practically impossible, at least for me, to foresee all these changes, optimizations, or whatever in advance.
If we don’t take the opportunity to refactor and improve while we have the funding, we might as well end up in endless
discussions and consume all those precious resources without anything concrete as outcome.
My proposal is to split the current proposed work in two phases:
- A first GSIP (and PR) focused on the points listed in the motivations/proposal, that is an upgrade of the libraries, and partial rewriting of the code/configurations strictly needed to make the new libs work. That GSIP would be reviewed, tested, and accepted quite quickly, because this is a much-needed improvement with a well-defined scope, and the current maintainer would know the existing dependencies between modules.
That’s not a bad idea at all, thinking of the 3.x series. I can explore it but definitely need
to reach a compromise to:
- A second GSIP about the module refactoring, that IMHO should be agreed upon by the developers’ community (and in particular module maintainers) in advance, to explore the possible alternative design, and find one that both parties agree upon. It would need more time to be investigated, tested, discussed, and in the end adopted and would give time to the other devs to get accustomed to it.
That would be this one. Given I’m committed to it I’d rather get the go ahead to have a 4.x branch or be forced to work on it from my fork alone.
Thanks again for the review and comments, looking forward to yours and anyone else’s.
Cheers,
Gabe.
Looking forward in reading your feedback.
Cheers,
Emanuele[1] https://github.com/geoserver/geofence/issues/128
[2] https://github.com/geoserver/geofence/wiki/GeoFence-REST-APIHi list,
I’ve just created a GSIP (216) with a proposal to make several improvements to GeoFence.
Please see https://github.com/geoserver/geoserver/wiki/GSIP-216 for details.
camptocamp
INNOVATIVE SOLUTIONS
BY OPEN SOURCE EXPERTS
Gabriel Roldán
Geospatial Developer
Gabe:
My initial feedback is - wow that is a big risk doing a month of work without having GSIP on the table for discussion on technical direction.
Point of the proposal process is to reduce risk for camptocamp and community interaction to avoid being left with either a lot of re-work or a fork as indicated by your 4.x version number 
In the future try to align GSIP process with some kind of design milestone for your customer; so you can get confirmation from your customer and the community before sinking a ton of budget into activity. It could be you found it easer to do a proof-of-concept and then share the design for discussion; but I wish to ensure you have budget for collaboration and compromise in reserve. Basic project management stuff for both geoserver and Camptocamp I admit; but it is important to control risk using the communication tools we have.
For the actual proposal I am presently -0; since I do not feel knowledgeable on geofence and do not have active customers using the technology. It would be much easier for collaboration if each one of the risks you mentioned were his own GSIP; as the all-or-nothing thing makes discussion difficult to follow.
If I understand:
Looks like it may be some automatic code generation? Hard to see
What is the impact of upgrading to a newer version? Migration challenges for existing users to address?
reading: I see you identified h2 upgrade as a problem. I think this is a key upgrade that is blocking work 
What is impacted by this? Is it just the integration between geoserver and geofence or is there more functionality (such as GeoNode or MapStore) that requires care?
reading: I see you acknowledge the REST API change. Is it possible to run both a v1 and a v2 rest api concurrently? We will need to hear from the community what the impact of this change is. It is always hard when your work impacts others as you need to ensure they have budget to proceed with the change.
Repository adapters? Not sure I follow the approach.
–
Jody Garnett
Hey Jody,
On Fri, Mar 10, 2023 at 4:04 PM Jody Garnett <jody.garnett@anonymised.com> wrote:
Gabe:
My initial feedback is - wow that is a big risk doing a month of work without having GSIP on the table for discussion on technical direction.
I think I understand the risk. Bear with me that’s the reason why I asked to create a GSIP,
being the first one related to GeoFence other than the one to promote the GeoServer plugins
from community to extension (GSIP-164), where 3 of 4 positive votes are from Geosolutions PSC members.
It is hard enough to jump into a project. I had my fears about the overall solution and hence needed
more than a quick POC to be sure everything would work. The ones who know the codebase will understand this:
If RuleReaderServiceImpl doesn’t work, then the work isn’t any good, since that one kind of gathers everything
together (authorization, rule and adminrule management, and resolving user roles). So the more I tried to keep
it focused, the deeper the rabbit hole.
This is by no means a complaint, it’s just a fact that 100% of commits to GeoFence that have any functionality
come from Geosolutions folks that never needed to make a GSIP for a change.
Point of the proposal process is to reduce risk for camptocamp and community interaction to avoid being left with either a lot of re-work or a fork as indicated by your 4.x version number
In the future try to align GSIP process with some kind of design milestone for your customer; so you can get confirmation from your customer and the community before sinking a ton of budget into activity.
Camptocamp delegates on me to set the technical direction on this particular endeavor.
None of the two customers involved care if we use GeoFence, one for sure doesn’t even
care if it’s Open Source, as long as it solves the problem. So I’m going through great lengths for it to be.
It could be you found it easer to do a proof-of-concept and then share the design for discussion; but I wish to ensure you have budget for collaboration and compromise in reserve. Basic project management stuff for both geoserver and Camptocamp I admit; but it is important to control risk using the communication tools we have.
At the same time, given I’ll be in charge of project maintenance for these customers for the foreseeable
future, I’d rather work on a codebase that’s closer to pleasure to work on than to a hassle. Again, no
offense, but it is what it is. And so I’m just asking for the minimum possible which is a branch, and offering
my commitment to work on every and any concern, be it related to design, architecture, or anything, at any pace,
because now I can, before I couldn’t. It would have been a ton of unknowns.
For the actual proposal I am presently -0; since I do not feel knowledgeable on geofence and do not have active customers using the technology. It would be much easier for collaboration if each one of the risks you mentioned were his own GSIP; as the all-or-nothing thing makes discussion difficult to follow.
If I understand:
- Google’s Generic DAO Framework → what is the proposed solution for this?
From a design point of view, we have Services that need to fetch rules from the persistence
layer using filter predicates.
GeoFence has a RuleFilter class, but the services used Google’s Generic DAO to to build DAO’s
layer predicates.
The proposal is to use RuleFilter itself as the predicate language for the persistence abstractions,
and let the implementations translate RuleFilter to native predicates as they wish.
e.g.:
RuleAdminService → RuleRepository.findAll(org.geoserver.geofence.filter.RuleFilter)
instead of:
RuleAdminService → RuleDAO.search(com.googlecode.genericdao.search.ISearch)
Looks like it may be some automatic code generation? Hard to see
From the implementation point of view, there are two RuleRepository implementations.
One works against the database using JPA, and maps RuleFilter to a com.querydsl.core.types.Predicate,
which is converted to an SQL/JPA query at runtime.
For this one we could also translate RuleFilter to Hibernate’s Criteria API instead, which is lacking in terms of
type safety, but can be done.
The other RuleRepository implementation is backed by a REST API client. The client itself is code-generated
from the OpenAPI spec. The RuleRepository implementation maps payloads from the API object model to the business
object model, and HTTP error codes to expected Exception types
.
Hence you can run RuleAdminService and RuleReaderService in the geoserver plugins hitting either the db,
or a remote GeoFence instance.
- Hibernate Spatial 1.1.3.2 replacement → upgrade
What is the impact of upgrading to a newer version? Migration challenges for existing users to address?
reading: I see you identified h2 upgrade as a problem. I think this is a key upgrade that is blocking work
The challenge is the H2 database upgrade. I think that’d be why Emmanuele’s branch is three years old.
- REST API differences between standalone and embedded → definition of a new OpenAPI 3.0 interface
What is impacted by this? Is it just the integration between geoserver and geofence or is there more functionality (such as GeoNode or MapStore) that requires care?
My understanding from the PMC meeting the other day, I might be wrong, is anyone is hardly using the standalone version of the REST API, so much that Andrea mentioned they thought of ditching it and just keep the one provided by GeoServer.
reading: I see you acknowledge the REST API change. Is it possible to run both a v1 and a v2 rest api concurrently? We will need to hear from the community what the impact of this change is. It is always hard when your work impacts others as you need to ensure they have budget to proceed with the change.
Yes, that’s the proposal, to let the GeoServer plugin run both v1 and v2. In any case, it’d use v2 to communicate with a remote GeoFence
when using the remote plugin (to replace Spring RMI), but whether to expose the v2 can be a configuration choice. Meanwhile,
anyone can use the OpenAPI specification to code-generate clients and upgrade to v2 at their own pace.
- Use of spring RMI →
Repository adapters? Not sure I follow the approach.
As explained above. The idea is to get rid of Spring RMI as it’s deprecated, and use the REST API to fetch data from Geofence, while eating our own dogfood.
Cheers,
Gabe
–
Jody GarnettOn Tue, Mar 7, 2023 at 6:34 PM Gabriel Roldan <gabriel.roldan@anonymised.com> wrote:
Hi list,
I’ve just created a GSIP (216) with a proposal to make several improvements to GeoFence.
Please see https://github.com/geoserver/geoserver/wiki/GSIP-216 for details.
camptocamp
INNOVATIVE SOLUTIONS
BY OPEN SOURCE EXPERTSGabriel Roldán
Geospatial Developer
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
Hi Gabriel,
I’m going to start briefly and do what I haven’t done in ages: cast a -1 on the proposal, but with an alternative proposal that will allow you to retain all your existing work.
Our community has had, over the years, a strong “talk first” policy. It means that one should discuss the design of a major change before doing actual work. This has precedent, for example it’s what I did in 2013, when I proposed to rewrite the CSS styling engine in Java (hard lesson learned there).
Another bit that is quite relevant, is who is in control of the module: the PSC is the maintainer of all core modules, but extensions modules have a dedicated maintainer that controls the module. In this case, that’s Emanuele. While for a situation like this it’s great to have a document that details the proposed changes, I believe the PSC role is mostly a consultation. The rules are not clear here, I’m open to discussion.
There are also practical aspects that make this rewrite a no go; they are not technical, but relate to pre-existing knowledge, availability of time and money. Maintenance of a module requires few things these days: being ready to review changes made by third parties, evaluate bug reports, and answer questions on the user list, on a “as time allows” basis. This can be done leveraging knowledge of how the module works.
The fork is reshuffling the entire code base, and introducing a number of libraries that are not part of the existing GeoServer dependencies. Picking up all these new libraries is problematic for the community at large, because it increases the number of bits the developers have to be familiar with, and makes the review of changes even harder, because one would have to get up to speed with them before even starting to look at the code.
Even assuming in blind faith that we could catch up later, it would still mean we have no way to review the changes in the short term (too much effort combined required to go and learn what we need, and then review the full rewrite).
So, where to go from here? Where you are going, we simply cannot follow.
We kindly ask you to retain all your work, and give it a different name: let’s call it GING for now (GING Is Not GeoFence).
The following points are very important for us:
GING is not an “upgrade” to GeoFence, but a fork of it, that users can migrate to.
GING is not endorsed by GeoSolutions, but has, at the beginning, Gabriel Roldan as the sole maintainer.
GING starts as a new community module and works its way up to extension status like any other module (including Geofence) did.
There is a number of advantages going this way:
Gabriel does not lose the large work investment in his fork, and gets a module that he’ll “be in charge of project maintenance for these customers for the foreseeable future” and will get a “codebase that’s closer to pleasure to work on than to a hassle”.
There is no longer reason to carry around baggage, Gabriel can drop the other REST APIs and just keep the newer one.
The H2 dependency is no longer a problem, it can be dropped right away.
Gabriel still gets documentation and UI that he can adapt to the “GING” name, without having to recreate all of that from scratch.
The developer community avoids a fight, and the user community gets two alternative security subsystems that they can choose from based on the functionality and level of support they get from their respective maintainers.
Finally, I’m not here saying that GeoSolutions will go with GeoFence in the longer term. It might well be that eventually the two end up converging, or that one eventually loses steam and dies and we end up all working on the same module. But in the short to medium term (e.g., the year to come), the direction proposed by Gabriel is not working for us. At the same time, the existing GeoFence code is not working for him, Emanuele has made a counter-proposal that has been rejected (or so I read the answer).
Thus, it’s best to take two separate ways, while still working among the same community, leveraging the pluggable nature of the GeoServer authorization subsystem to keep both modules around.
Best regards
Andrea
Regards,
Andrea Aime
==
GeoServer Professional Services from the experts!
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions Group
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
https://www.geosolutionsgroup.com/
http://twitter.com/geosolutions_it
Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia.
This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail
Hi Andrea, Emanuele, and all.
Thanks for this counter-proposal. It makes a lot of sense, and at Camptocamp we just met and decided it’s a good way forward.
The project will be called GeoServer ACL, as in Access Control List.
It will have a somewhat more limited scope than GeoFence, at least initially,
focusing only on a separate service, not an embedded engine.
Therefore, we need to fine tune a couple aspects.
First and foremost, we need a home for it. It could be as part of geoserver-cloud’s codebase,
but it’d be so much better if we can be granted a /geoserver/geoserver-acl
github repository.
Additionally we’ll need to be able to deploy artifacts to maven, so the community
module can depend on them.
Do you think those two requests are acceptable? How can we proceed? is a GSIP
required to create a sibling project under github’s /geoserver organization?
Best regards,
camptocamp
INNOVATIVE SOLUTIONS
BY OPEN SOURCE EXPERTS
Gabriel Roldan
Geospatial Developer
Update to GSIP with additional details, such as adding a repo.
Personally I would like these other geoserver repos to match geosever release cycle from a QA standpoint.
–
Jody Garnett
That is fine by me. Just thinking out loud, in terms of having the least amount of moving parts, since GeoSever cloud is already depending
on GeoServer, would it make sense to have a GeoServer community module with all the “library” parts, as well as a module generating
a Spring Boot jar for stand-alone running, and have GeoServer Cloud wrap it to handle the kubernetes integration?
As said… just wondering.
Cheers
Andrea
Regards,
Andrea Aime
==
GeoServer Professional Services from the experts!
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions Group
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
https://www.geosolutionsgroup.com/
http://twitter.com/geosolutions_it
Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia.
This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail
Hi Andrea,
On Thu, Mar 16, 2023 at 1:37 PM Andrea Aime <andrea.aime@anonymised.com> wrote:
That is fine by me.
Okay cool.
Just thinking out loud, in terms of having the least amount of moving parts, since GeoSever cloud is already depending
on GeoServer, would it make sense to have a GeoServer community module with all the “library” parts, as well as a module generating
a Spring Boot jar for stand-alone running, and have GeoServer Cloud wrap it to handle the kubernetes integration?
I could be technically possible with some hiccups, like test dependencies added to all projects from the root pom
causing some headaches, maybe spring version mismatches too, pegging the lifecycle of the application to that of
a community module with all that implies in terms of build server etc.
But most importantly, being a separate library and application from both geoserver and geoserver cloud, it’d be
better off on its own repo.
Cheers,
Gabe
As said… just wondering.
Cheers
AndreaOn Thu, Mar 16, 2023 at 3:57 PM Gabriel Roldan <gabriel.roldan@anonymised.com> wrote:
Hi Andrea, Emanuele, and all.
Thanks for this counter-proposal. It makes a lot of sense, and at Camptocamp we just met and decided it’s a good way forward.
The project will be called GeoServer ACL, as in Access Control List.
It will have a somewhat more limited scope than GeoFence, at least initially,
focusing only on a separate service, not an embedded engine.Therefore, we need to fine tune a couple aspects.
First and foremost, we need a home for it. It could be as part of geoserver-cloud’s codebase,
but it’d be so much better if we can be granted a /geoserver/geoserver-acl
github repository.Additionally we’ll need to be able to deploy artifacts to maven, so the community
module can depend on them.Do you think those two requests are acceptable? How can we proceed? is a GSIP
required to create a sibling project under github’s /geoserver organization?
Best regards,
camptocamp
INNOVATIVE SOLUTIONS
BY OPEN SOURCE EXPERTSGabriel Roldan
Geospatial DeveloperOn Mon, Mar 13, 2023 at 12:14 PM Andrea Aime <andrea.aime@anonymised.com> wrote:
Hi Gabriel,
I’m going to start briefly and do what I haven’t done in ages: cast a -1 on the proposal, but with an alternative proposal that will allow you to retain all your existing work.Our community has had, over the years, a strong “talk first” policy. It means that one should discuss the design of a major change before doing actual work. This has precedent, for example it’s what I did in 2013, when I proposed to rewrite the CSS styling engine in Java (hard lesson learned there).
Another bit that is quite relevant, is who is in control of the module: the PSC is the maintainer of all core modules, but extensions modules have a dedicated maintainer that controls the module. In this case, that’s Emanuele. While for a situation like this it’s great to have a document that details the proposed changes, I believe the PSC role is mostly a consultation. The rules are not clear here, I’m open to discussion.
There are also practical aspects that make this rewrite a no go; they are not technical, but relate to pre-existing knowledge, availability of time and money. Maintenance of a module requires few things these days: being ready to review changes made by third parties, evaluate bug reports, and answer questions on the user list, on a “as time allows” basis. This can be done leveraging knowledge of how the module works.
The fork is reshuffling the entire code base, and introducing a number of libraries that are not part of the existing GeoServer dependencies. Picking up all these new libraries is problematic for the community at large, because it increases the number of bits the developers have to be familiar with, and makes the review of changes even harder, because one would have to get up to speed with them before even starting to look at the code.
Even assuming in blind faith that we could catch up later, it would still mean we have no way to review the changes in the short term (too much effort combined required to go and learn what we need, and then review the full rewrite).
So, where to go from here? Where you are going, we simply cannot follow.We kindly ask you to retain all your work, and give it a different name: let’s call it GING for now (GING Is Not GeoFence).
The following points are very important for us:
GING is not an “upgrade” to GeoFence, but a fork of it, that users can migrate to.
GING is not endorsed by GeoSolutions, but has, at the beginning, Gabriel Roldan as the sole maintainer.
GING starts as a new community module and works its way up to extension status like any other module (including Geofence) did.
There is a number of advantages going this way:Gabriel does not lose the large work investment in his fork, and gets a module that he’ll “be in charge of project maintenance for these customers for the foreseeable future” and will get a “codebase that’s closer to pleasure to work on than to a hassle”.
There is no longer reason to carry around baggage, Gabriel can drop the other REST APIs and just keep the newer one.
The H2 dependency is no longer a problem, it can be dropped right away.
Gabriel still gets documentation and UI that he can adapt to the “GING” name, without having to recreate all of that from scratch.
The developer community avoids a fight, and the user community gets two alternative security subsystems that they can choose from based on the functionality and level of support they get from their respective maintainers.
Finally, I’m not here saying that GeoSolutions will go with GeoFence in the longer term. It might well be that eventually the two end up converging, or that one eventually loses steam and dies and we end up all working on the same module. But in the short to medium term (e.g., the year to come), the direction proposed by Gabriel is not working for us. At the same time, the existing GeoFence code is not working for him, Emanuele has made a counter-proposal that has been rejected (or so I read the answer).
Thus, it’s best to take two separate ways, while still working among the same community, leveraging the pluggable nature of the GeoServer authorization subsystem to keep both modules around.Best regards
AndreaOn Fri, Mar 10, 2023 at 6:48 PM Gabriel Roldan <gabriel.roldan@anonymised.com757…> wrote:
Hi Emmanuele,
Thanks for the proposal review and comments.
First and foremost, let me stress out the proposal is to create a branch, 4.0.x.
Since there are several topics that of course need to be discussed and agreed
upon, all this work would serve as a concrete platform to do that, with as much
proof as possible that everything keeps working.
Not to merge or force any of this in the current 3.x series.Follow up inline.
On Thu, Mar 9, 2023 at 12:40 PM Emanuele Tajariol <emanuele.tajariol@anonymised.com887…> wrote:
Dear Gabriel,
I went through your proposal; I will start with some comments about the motivation section:
- The persistence abstractions are powered by Google’s Generic DAO Framework, which is not maintained since years ago
- This refactoring was already completed and working, as reported in this issue:
- JDK11 compatibility #128 [1]
Oh that’s interesting. Unfortunately I couldn’t know, or at least it was easy to oversight,
since that issue is three years old, it’s not merged, and the title does not relate to the persistence
framework; though the description does indeed mention it. Should I’ve noticed it, it would have
saved me a ton of work.
- All the GenericDAO constructs were ported to Hibernate’s criteria and to some newly created classes to reduce the impact on the existing code as much as possible.
Yes, the Hibernate criteria API also makes sense.
Personally, I prefer QueryDSL for the type-safety part. QueryDSL code-generates classes that
let you create arbitrary queries in a type safe way, and adds no runtime dependencies, just a
compile-time @Entity annotation processor.
In any case this and everything else is up to discussion.
- The REST APIs exposed by GeoFence’s standalone server application, and the GeoFence embedded engine in GeoServer, have some differences, which makes it hard to use a REST client interchangeably.
- When the REST APIs were reimplemented in the embedded configuration (in the geofence-server module), they did not follow the detailed documentation available at GeoFence REST API [2] , that was followed in the initial (standalone) implementation. I would not create a third API, but would stick to the existing one: breaking backwards compatibility is something we try very hard not to do in GeoServer, since there are deployed systems depending on it that would break on upgrade.
As much as I very much agree with the principle, we’re kind of in an impossible situation here.
I agree having two disparate REST APIs is bad enough.On the other hand, I have been working for a year now on a system that needs to interface with GeoFence.
I would rather have the chance to deploy GeoFence standalone or embedded, and be able to use the same
REST API regardless, but that’s impossible without tons of hacks.It’s also difficult to implement a client even if you choose one of them, since there are some inconsistencies like
requests to create a Rule accepting a given content type (e.g. application/json) but returning text/plain with the
rule id, and then expecting long to fetch the rule.Additionally, in order to replace Spring RMI with an API client, the client would need to implement GeoFence standalone’s
REST API, but a client working against GeoServer would implement GeoFence embeeded’s REST API, so we’re just multiplying
the maintenance problem.So my proposal is to:
1- use an API-first approach through an OpenAPI specification that’s common to both kinds of deployments.
2- Maintain the current embedded API for as long as necessary in GeoServer.
3- Also expose the new API as v2, where anyone can use the provided or generate ready to use client libraries in different programming languages(I’ve actually just finished porting both geoserver plugins, will send a link once I get the configuration part working too in a backward compatible way)
Thinking of an upgrade path:
For GeoServer embedded, REST API wise, wouldn’t make any difference, other than v2 also being available, or not even, could be a configuration choice.
For standalone, an upgrade would also mean a standalone GeoFence upgrade, so it doesn’t matter if it uses RMI or the new OpenAPI interface to communicate with GeoFenceRegarding the other points, it’s great getting rid of the older libraries (we had to revive the old Hibernate Spatial in order to keep up with the JTS upgrades – see https://github.com/geosolutions-it/hibernate-spatial).
So, apart from the notes above, I’m good with the various motivations and points in the proposal, because they aim to get rid of older unmaintained libraries and to keep the libs aligned with the ones used in GeoServer.nice!
That said, I’m quite concerned with the modules refactoring:
- They are not strictly needed and motivated by the previous points.
While I disagree (again, for a longer term, on 4.x) , I see my mistake in not emphasizing
on the motivation section and only mentioning sharing the maintenance burden in the
overview section, I guess I just didn’t want to come across as arrogant or patronizing.Since we’re kicking off the discussion, allow me to elaborate:
I know both Alessio and yourself shall know the codebase from the heart, and that’s great,
but bear with me, as an outsider and newcomer to the project, I had a
rather hard time getting acquainted with it, hence my intention is to make sure the architecture
is very clear and upfront, not only for us but for any other potential collaborators. Something
closer to a Screaming Architecture, as my initial struggles were related to figuring out the architecture,
and how components relate to each other, not from the persistence viewpoint but from
the business domain view.If you look at the following comparison of 3.x and 4.x proposed modularity, I think the 3.x one doesn’t
say much about what the software system is or does, but instead I see there are apis, persistence, and
a gui.
The 4.x proposal on the other hand makes it very clear that the business domain has 4 domain units
and a common object model, and that’s all one would need to look into to understand GeoFence.
No Spring, no JPA, no REST, no anything else than pure domain specific logic.
Everything else is integration and implementation detail you can dig into if need be.
- They will likely make it difficult to review and functionally validate.
They might. Hence all functional tests are in the domain modules themselves, without requiring digging
into integration details, while at the same time serve as base integration tests for both the persistence
and client api integrations.
The tests (e.g. the most important being RuleReaderServiceImplTest) run against in-memory
persistence abstraction implementations that use RuleFilter itself as predicate, through the
new RuleFilter.matches(Rule):boolean method, and that helped me a lot to make sure both the openapi
client and JPA backed Repositories do the right thing and are hence interchangeable as backing
Repositories for the different services.Other things like making the object model immutable also helps to clearly reason about things, like in
avoiding side effects with argument objects being changed by functions.
- They may introduce further issues, along with the potential ones caused by the already big library upgrade shock.
Yes they would, as there’s no such thing as bug-free software is it? On the bright side we will be taking on the burden of stress
testing and fixing for our current customer, and another one in the very near future.
- They may be a blocker for the current maintainers to keep up the work and to respond quickly to the issues that will pop up, especially in the first times it will be released.
Indeed. So once again, no rush. I’m only asking for a branch so we have a common place to review, discuss, and
coordinate on things. I honestly think having this starting point is much better than trying to make one proposal at a time
because it’s practically impossible, at least for me, to foresee all these changes, optimizations, or whatever in advance.
If we don’t take the opportunity to refactor and improve while we have the funding, we might as well end up in endless
discussions and consume all those precious resources without anything concrete as outcome.My proposal is to split the current proposed work in two phases:
- A first GSIP (and PR) focused on the points listed in the motivations/proposal, that is an upgrade of the libraries, and partial rewriting of the code/configurations strictly needed to make the new libs work. That GSIP would be reviewed, tested, and accepted quite quickly, because this is a much-needed improvement with a well-defined scope, and the current maintainer would know the existing dependencies between modules.
That’s not a bad idea at all, thinking of the 3.x series. I can explore it but definitely need
to reach a compromise to:
- ditch the GWT gui and provide only a spring-boot application for the current (standalone) REST API. Or maybe
not even, does anyone use it? or does everyone uses the embedded engine?- keep on using RMI for GeoServer to GeoFence communication
- A second GSIP about the module refactoring, that IMHO should be agreed upon by the developers’ community (and in particular module maintainers) in advance, to explore the possible alternative design, and find one that both parties agree upon. It would need more time to be investigated, tested, discussed, and in the end adopted and would give time to the other devs to get accustomed to it.
That would be this one. Given I’m committed to it I’d rather get the go ahead to have a 4.x branch or be forced to work on it from my fork alone.
Thanks again for the review and comments, looking forward to yours and anyone else’s.
Cheers,
Gabe.Looking forward in reading your feedback.
Cheers,
Emanuele[1] https://github.com/geoserver/geofence/issues/128
[2] https://github.com/geoserver/geofence/wiki/GeoFence-REST-APIOn Wed, Mar 8, 2023 at 3:35 AM Gabriel Roldan <gabriel.roldan@anonymised.com> wrote:
Hi list,
I’ve just created a GSIP (216) with a proposal to make several improvements to GeoFence.
Please see https://github.com/geoserver/geoserver/wiki/GSIP-216 for details.
camptocamp
INNOVATIVE SOLUTIONS
BY OPEN SOURCE EXPERTSGabriel Roldán
Geospatial Developer
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel–
Regards,
Andrea Aime
==
GeoServer Professional Services from the experts!Visit http://bit.ly/gs-services-us for more information.
Ing. Andrea Aime
@geowolf
Technical LeadGeoSolutions Group
phone: +39 0584 962313fax: +39 0584 1660272
mob: +39 339 8844549
https://www.geosolutionsgroup.com/
http://twitter.com/geosolutions_it
Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia.
This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail
–
Regards,
Andrea Aime
==
GeoServer Professional Services from the experts!Visit http://bit.ly/gs-services-us for more information.
Ing. Andrea Aime
@geowolf
Technical LeadGeoSolutions Group
phone: +39 0584 962313fax: +39 0584 1660272
mob: +39 339 8844549
https://www.geosolutionsgroup.com/
http://twitter.com/geosolutions_it
Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia.
This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail
Works for me. Since we are talking about a new repo, I’d say the PSC should have a say.
How about you amend the existing GSIP with a description of the new work and repo, and we can vote
on it?
Cheers
Andrea
Regards,
Andrea Aime
==
GeoServer Professional Services from the experts!
Ing. Andrea Aime
@geowolf
Technical Lead
GeoSolutions Group
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549
https://www.geosolutionsgroup.com/
http://twitter.com/geosolutions_it
Con riferimento alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni circostanza inerente alla presente email (il suo contenuto, gli eventuali allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le sarei comunque grato se potesse darmene notizia.
This email is intended only for the person or entity to which it is addressed and may contain information that is privileged, confidential or otherwise protected from disclosure. We remind that - as provided by European Regulation 2016/679 “GDPR” - copying, dissemination or use of this e-mail or the information herein by anyone other than the intended recipient is prohibited. If you have received this email by mistake, please notify us immediately by telephone or e-mail