cf-java-client icon indicating copy to clipboard operation
cf-java-client copied to clipboard

Lacking v3 APIs

Open Suyash08Oct opened this issue 2 years ago • 30 comments

Hello, As a consumer of this cloud foundry client, I noticed that for v3 some of the APIs are lacking, which includes some of the major functional apis for operations related to apps, service instance, service plans, service keys, service brokers,space routes,security groups etc.

I would like to contribute here, to bridge these gaps from cf v2 to v3. Please help me with the procedures for the same. and please acknowledge if these changes are already in pipeline to get released.

And please share the release process and the estimated time taken for the PR approval, review and release. So that as a consumer, I can also plan to consume all the v3 related APIs in my application.

Attachment: captured some of the gaps from v2 to v3 lacking v3 apis

Suyash08Oct avatar Jul 21 '22 16:07 Suyash08Oct

First, let me say thanks for taking the time to put this list together. Some of these gaps were known, some were not.

Second #1157 which was just merged, should I believe complete a few of these (service instances & plans I believe). I think #1158 may cover some as well, but I haven't look in depth at that PR yet.

As far as contributing PRs, we'd love to have them.

  1. You have to sign the CLA before we can accept any PRs. The first PR you submit will prompt you. If you work for a large company you may already be covered. The bot will walk you through the process.
  2. Any PR to add new APIs should contain the client, reactor client implementation, reactor client unit tests, and integration tests to validate the work. The integration tests part can be a little tricky as you need admin access to a CF environment to run them. This link has some pointers for running the tests and getting an environment. I have also offered to run integration tests for folks, as we have labs to run them. I would just ask that your tests compile OK first & you are reasonably confident they'll work if you want to go this route. That'll reduce the roundtrips it takes to get things ready to merge. #1157 is a good example.
  3. As far as turn-around time. We try to take an initial look at the PR within a week of being submitted. The time to merge will vary depending on if any changes or corrections are required. Times can vary as the small team at VMware that manages this project is also responsible for a number other projects and from time to time we get delayed by work on other projects. We do try to be upfront in the PR about this when it happens. We appreciate your contributions and want to respect your time as we know it's valuable.

Thanks & let me know if you have any other questions.

dmikusa avatar Jul 21 '22 17:07 dmikusa

Thank you so much for the quick and detailed reply. Regarding PR Release, I can understand the constraints. I was following up on the time line for the release because we are currently consuming this client with the v2 version. We are planning to shutdown the use of v2 APIs . So we need to perform migration from v2 to v3 as early as possible. So if will get the rough estimates for the complete process (from PR till release) required for the release for our PRs, it will be easy for us to plan our consumption accordingly.

Suyash08Oct avatar Jul 25 '22 07:07 Suyash08Oct

We are planning to shutdown the use of v2 APIs

Could you expand on this? Is that an internal choice? While CAPI v2 is deprecated, I've heard no plans or timeline around the actual removal of the v2 API.

So if will get the rough estimates for the complete process (from PR till release) required for the release for our PRs, it will be easy for us to plan our consumption accordingly.

Our present coverage, as you've seen, is not complete for the v3 API however we do have quite a few of the APIs working. If you need one of the missing APIs, feel free to submit a PR & we can typically get that reviewed and merged within a few weeks. Releases may take a little longer, but it really just depends on what is happening at the moment. We like to batch up multiple changes in a release where possible. At any rate, I would just encourage you to mention/ask at the time on the issue/PR you open and we can figure out a reasonable course of action for a release.

Where we are more lacking is in the operations library, not sure if you're using that library, but the operations library provides more high-level concepts like cf push, and most of the v2 operations library is still using v2 APIs. This still works because the v2 API is still present and operating, but there is some new functionality that's missing, like rolling deployments & rollbacks. You can technically do that with the v3 client API, but it's more work.

Hope that's helpful. Let me know if you have any additional questions.

dmikusa avatar Jul 27 '22 19:07 dmikusa

Some background info regarding the mentioned "shutdown the use of v2 APIs":

As you said, there are no plans to remove the v2 API implementation from CAPI.

However, we (SAP) plan to reduce (drastically) the CF API v2 usage on our Cloud Foundry foundations. The main reason is that the v2 API doesn't perform that well on large foundations (think of > 70T used app memory, > 150 api VMs). We fear that we hit CF API scalability limits on further growth. Our metrics show that an avg v2 request takes more than twice the time of an avg v3 request. And all investments in CAPI performance and scalability go into v3 as v2 is deprecated.

We won't switch off v2 completely in our foundations but we will add an additional v2 rate limiting and decrease the limit over time: https://github.com/cloudfoundry/cloud_controller_ng/pull/2882

stephanme avatar Jul 29 '22 16:07 stephanme

@stephanme Thanks. That makes perfect sense.

dmikusa avatar Jul 29 '22 17:07 dmikusa

Hello @dmikusa,

To initiate the process for contributing the lacking v3 API, I have forked the source code. And was going through it in detail. I have gone through https://github.com/cloudfoundry/cf-java-client#readme for the details. Is there any architectural document, which I can follow for the deep insights of the source code.

Thanks, Suyash

suyash-sap avatar Nov 23 '22 11:11 suyash-sap

That's really it for documentation.

At a high level, you have the client which is generic, you have an implementation of the client which is written using Reactor, and the operations module which is higher-level operations built on top of the client, these are things that take more than one call to do, like cf push.

dmikusa avatar Nov 23 '22 17:11 dmikusa

Thanks for the explanation.

In client, written with Reactor, there is a fine segregation between v2 and v3 implementations. But with cloudfoundry-operations, I am unable to find the same.

For example, If i need to do cf create service test then the call will eventually come under DefaultServices.java which is calling the v2 specific client implementation.

code

I need to understand the architectural design which we need to follow here, so that we can make the operations ready for consuming both v2 and v3 specific clients.

Please feel free to correct my understanding, If I got it wrong.

Thanks, Suyash

suyash-sap avatar Nov 24 '22 05:11 suyash-sap

In client, written with Reactor, there is a fine segregation between v2 and v3 implementations. But with cloudfoundry-operations, I am unable to find the same.

Correct, there is no separation of the code in operations. I think you could perhaps say that there doesn't need to be a difference, at least not per API version. The operations library provides high-level operations, which take multiple client calls. If you're using v2 or v3 or a mix of v2 and v3, it doesn't really matter to the consumer of the operations library. They just want the operation to work.

Where that gets tricky is when you can't do something in the same way with v2 API as you can with the v3 API. The cf push operation comes to mind here. I'm not totally sure that you could swap out v2 for v3 API and not have a breaking change. If there are differences in behaviors, we'd need to account for that somehow, probably with a new method or operation.

I'm not opposed to doing it differently though if folks think that it would be better to have it versioned. That would require a new structure in the code to differentiate things by API version like you have with the client. That doesn't exist in operations because I don't think any of the operations have started to use v3 yet.

Hope that helps!

dmikusa avatar Nov 25 '22 04:11 dmikusa

Both of the Approaches which is 1. Using v3 API directly from operations and 2. Creating a separate layer for v3 has their pros and cons, which I feel should be discussed before:

For Approach 1: Using v3 API directly from operations Pros:

  • Ease in development as it will have minor changes in the current cloudfoundry-operations.
  • Ease in consuming the cf-java-client for existing consumers, as method will be same, the change will be internal to operations for v3 consumption.

Cons:

  • The current design of the client has a fine segregation between v2 and v3, So changing the operations using v3 will not be as per the current architecture and it will limit the v2 to the rector client specific implementation only.
  • Is the partial implementation of cloudfoundry-operations will work and can get released? where some of the methods will use v2 and some will refer to v3 for their implementations.

For Approach 2: Creating a separate layer for v3 in cloudfoundry-operations Pros:

  • Aligned Implementation with the current architecture and has a fine layer segregation.
  • Consumers has an option to switch to the specific implementations.

Cons:

  • Code and Efforts will get Duplicated.
  • With the obsolescence of v2, the code for v2 will not get used at all.

This is what I can think of now, we can have a brainstorm here for a decision so that, we can the start the contributions as early as possible.

Thanks, Suyash

suyash-sap avatar Nov 25 '22 09:11 suyash-sap

I don't really have a preference for approach 1 or 2. Each has some pros and cons, I'm not sure there's a clear "better" option.

Given that there is a difference in the level of effort for the implementation, I would leave the choice up to the individual that wants to implement and contribute the code change since it's their time and effort. In short, it's up to you @suyash-sap I'm Ok with either approach.

dmikusa avatar Nov 25 '22 21:11 dmikusa

With Approach 1, Is the partial implementation of cloudfoundry-operations will work and can get released? where some of the methods will use v2 and some will refer to v3 for their implementations.

suyash-sap avatar Nov 28 '22 10:11 suyash-sap

Yes, I think that would be an advantage of this approach. We can slowly upgrade the library and users will just get the benefit of switching to v3 without requiring any code changes. It just emphasizes the importance of making the upgrades in a non-breaking way. If we can't do that for a particular operation, then we'll have to version that particular operation. That's still a bit less work (for us and for users migrating) though than versioning the entire operations library.

dmikusa avatar Nov 28 '22 13:11 dmikusa

Thanks for the info @dmikusa.

We have completed the initial setup of the client. I was trying to configure the integration tests, for that we have created a cloud foundry instance.

Screenshot 2022-12-12 at 10 54 48 PM

I can login to this instance via sso or by my userName and password. For integration tests to run I will be needing CLIENT_ID and CLIENT_SECRET. How can we configure these two? If there is any docs for the same, which we can refer, It will be helpful.

suyash-sap avatar Dec 12 '22 17:12 suyash-sap

@suyash-sap Hi, I believe that the integration tests require admin access as they need to exercise all facets of the API, some of which require admin access.

At any rate, I suggest using the admin user and admin client that is created as a part of the CF install (at least all of the CF installs I've encountered have these). Both live in UAA.

dmikusa avatar Dec 12 '22 18:12 dmikusa

@dmikusa We are trying to run integration test, for that, We have enabled a cf org with admin access. We dont have CLIENT_ID and CLIENT_SECRET as our org has no service installed / bind there.

Do we need any plugin (or uaa), or any client installed in our org, which will give us the client_id and client_secret for authorization.

If we are running the integration test passing only three parameters which are TEST_ADMIN_PASSWORD, TEST_ADMIN_USERNAME, TEST_APIHOST, we are ending up in the error:

Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'cloudFoundryCleaner' defined in org.cloudfoundry.IntegrationTestConfiguration: Invocation of init method failed; nested exception is org.cloudfoundry.client.v2.ClientV2Exception: CF-NotAuthorized(10003): You are not authorized to perform the requested action

suyash-sap avatar Dec 21 '22 10:12 suyash-sap

Yes, you get that error because some of the actions that are run during the integration test require admin client permissions.

I believe that the admin client is going to be configured in your deployment manifest. I don't know if you can retrieve it from anywhere else. It is the same token you'd use to add admin users like in the docs here.

dmikusa avatar Dec 23 '22 14:12 dmikusa

Hello @dmikusa, We have tried the above option and it is not working in case of our specific cf setup. Would it be possible for you to run the integration tests in your labs, if we write and push the same. In case of failures, you can provide us the reports, which we will be fixing as a part of PR.

One more question, A part from integration test, is there any way we can test our implementation for the changes we are developing.

Thanks, Suyash

suyash-sap avatar Jan 03 '23 10:01 suyash-sap

@suyash-sap I don't have access to lab environments either. I believe @pivotal-david-osullivan does, so pulling him into this conversation.

dmikusa avatar Jan 03 '23 13:01 dmikusa

Hello @dmikusa,

We have initiated a small thread of contribution https://github.com/cloudfoundry/cf-java-client/pull/1174, https://github.com/cloudfoundry/cf-java-client/pull/1173. The motivation behind starting with a small contribution is to get familiar with the process. I am unable to add you as a reviewer. Please help me with the same.

One more question, A part from integration test, is there any way we can test our implementation for the changes we are developing.

Thanks, Suyash

suyash-sap avatar Jan 12 '23 07:01 suyash-sap

Hi @suyash-sap,

I am the maintainer of Promregator - a consumer of this library here. As @stephanme pointed out, your company wants to limit support of CAPI V2 significantly. I am aware of at least two consumers in your ecosystem which are using Promregator. So far, Promregator is running entirely on CAPI V2 - and hence would most probably go dark in case you shut V2 down.

With the most recent version of Promregator (only release candidates so far), I have already migrated all Promregator's V2-based operations of the library to V3-based operations. Out of that experience I can tell you that the approach with "mapping V2->V3 stuff" will become very tedious and complex: For example, replacing the call for the SpaceSummaryV2 endpoint will become a nightmare, as you will have to read data from (at least) three different other V3 endpoints to collect the necessary details - lest talking about the intrinsic complexity you will have to add to the client's logic to map everything together. Moreover, most consumers won't need every detailed information, so you will be creating a lot of "noisy requests" to the V3 endpoints without getting any benefit for it. The library's consumers also don't have an option to tell you that certain attributes within a request do not need to be requested, so you also cannot "switch them off dynamically". I suggest to check on this (and perhaps further use cases) before really continuing with the approach, as I expect you will run into more such cases. To me the current approach is questionable, as I also had to refactor some things (in particular caches and some reactor flux flow) to properly accommodate to the CAPI V3's structure.

So far, in normal operations, I am rather happy with my V3-migrated approach. To verify that I haven't overlooked anything, I changed the cloudfoundry-client's logging level to DEBUG and traced all outbound requests. To my surprise, although I had replaced all API-V2 occurrences in my own code, I still came across another occurrence. Here's an excerpt of the log:

2023-01-21T22:12:19.493+01:00 DEBUG 10324 --- [   scheduling-1] cloudfoundry-client.token                : Negotiating using token provider
2023-01-21T22:12:19.539+01:00 DEBUG 10324 --- [ry-client-nio-3] cloudfoundry-client.request              : GET    /
2023-01-21T22:12:19.616+01:00 DEBUG 10324 --- [ry-client-nio-3] cloudfoundry-client.response             : 200    / (77 ms)
2023-01-21T22:12:19.659+01:00 DEBUG 10324 --- [ry-client-nio-3] cloudfoundry-client.request              : GET    /v2/info
2023-01-21T22:12:19.962+01:00 DEBUG 10324 --- [ry-client-nio-3] cloudfoundry-client.response             : 200    /v2/info (303 ms)
2023-01-21T22:12:20.090+01:00 DEBUG 10324 --- [ry-client-nio-3] cloudfoundry-client.request              : POST   /cloudfoundry/login/uk-south/oauth/token
2023-01-21T22:12:20.868+01:00 DEBUG 10324 --- [ry-client-nio-3] cloudfoundry-client.token                : Refresh Token: ey***REDACTED**
2023-01-21T22:12:20.890+01:00 DEBUG 10324 --- [ry-client-nio-3] cloudfoundry-client.token                : Refresh Token Issued At:  2023-01-21T21:12:22 UTC
...

After having had a look into the internals of the library, I believe to have tracked the call down somewhere to the RootProviders around https://github.com/cloudfoundry/cf-java-client/blob/dcba939228fadb0922d73eae28d813ccae774efa/cloudfoundry-client-reactor/src/main/java/org/cloudfoundry/reactor/tokenprovider/AbstractUaaTokenProvider.java#L236 -- so that is in the area of UAA authentication management (not XSUAA, but UAA of the Cloud Controller). It appears that the client authentication logic calls the /v2/info endpoint to determine where the authorization endpoint of the UAA server is located.

Looking at the topic from a logical point of the API specification, you will notice that the /v3/info does not provide associations regarding the UAA server (see https://v3-apidocs.cloudfoundry.org/version/3.130.0/index.html#info).

@stephanme: What are your plans concerning removing CAPI V2 endpoints in detail? That is to ask: Are you planning to also throttle /v2/info? If yes, how will clients be able the determine the authorization_endpoint link in a compatible way but not falling into your rate limiting trap?

@Suyash08Oct Will you also consider this case in your refactoring activities?

Thanks for considering!

eaglerainbow avatar Jan 21 '23 21:01 eaglerainbow

@eaglerainbow : Thanks for your feedback.

Our plan is to apply rather strict rate limits on the CF API v2 instead of removing v2 completely. Think of a few hundred v2 requests per user per hour in contrast to a few 10k v3 req per user per hour. This still allows legacy v2 use cases like e.g. older CF CLI plugins but only at a usage rate that doesn't impact the cloud controller performance too much.

The /v2/info endpoint is not rate limited and will continue to work on our CF foundations. See ccng rate_limiter_v2_api.rb.

I think the better replacement for /v2/info is the root endpoint / (https://v3-apidocs.cloudfoundry.org/index.html#root, ) and not /v3/info (https://v3-apidocs.cloudfoundry.org/index.html#info). The root endpoint contains the authorization (UAA) endpoint information. Root and v3 info endpoint are not rate limited as well.

stephanme avatar Jan 23 '23 12:01 stephanme

The /v2/info endpoint is not rate limited and will continue to work on our CF foundations. See ccng rate_limiter_v2_api.rb.

Thanks for the clarification and also the confirmation! I wouldn't invest too much effort then in that.

I think the better replacement for /v2/info is the root endpoint / [...]

I agree, the UAA's servers address is mentioned there. But then it would be interesting to know why /v2/info gets called in first place - if / was already called before. Perhaps my initial analysis is incomplete and it's not just the UAA's server that is determined by this. At any rate, if the /v2/info endpoint is not rate-limited, then there is no issue with calling it altogether.

eaglerainbow avatar Jan 23 '23 19:01 eaglerainbow

Hello @dmikusa,

We have setup our integration test env, while running the integration test provided in the main branch, we are getting multiple errors which are org.cloudfoundry.reactor.util.JsonParsingException: Found unexpected property log_rate_limit in payload for org.cloudfoundry.client.v2.spaces.SpaceApplicationSummary$Json

Any idea how will we resolve this issue?

Note: We are getting this warning before running the integration test Client supports API version 2.180.0 and is connected to server with API version 2.197.0. Things may not work as expected.

suyash-sap avatar Feb 22 '23 13:02 suyash-sap

Hello @dmikusa, With respect to integration test env, we are facing one more issue while executing integration test for Applications API.

org.cloudfoundry.reactor.util.JsonParsingException: Found unexpected property refreshTokenRotate in payload for org.cloudfoundry.uaa.identityzones.TokenPolicy$Json

Any idea how will we resolve this issue?

Thanks.

abhishek7-sap avatar Feb 23 '23 09:02 abhishek7-sap

Just to be sure I got this conversation correctly:

While trying to get routes via the operations client, I encountered an error telling me rate limit for v2 has been exceeded, but switching to RoutesV3 in operations is not possible as its not yet implemented, correct?

Napfton avatar Mar 17 '23 06:03 Napfton

@suyash-sap @abhishek7-sap - Sorry, I'm not sure about the errors you're seeing. It sounds like your token might have a slightly different structure than what's expected in org.cloudfoundry.uaa.identityzones.TokenPolicy. I'm not sure why that would be. Maybe you could look at changing the serialization configuration to be permissive of extra fields? Maybe something here would help? https://www.baeldung.com/jackson-deserialize-json-unknown-properties

@Napfton Correct. The operations library is still mostly using v2 APIs. You can use the clients API directly, but that is a bit lower level, so depending on what you're trying to do it could be non-trivial.

dmikusa avatar Mar 17 '23 12:03 dmikusa

With regard to migrating the Operations classes to using V3 APIs, the Operations classes are modelled after the cf CLI. Since the cf CLI has migrated to use the V3 APIs, it seems only logical that we make the same change.

That being said, I try to avoid using the Operations classes as much as possible because the API is lacking in critical areas. (i.e. requires using a name and has no support for using a guid). Modeling the Operations classes, which are targetted at because consumed inside of an application, off of the cf CLI, which is targeted at being used by humans, was a huge mistake.

That being said, I would support efforts into a new V3 Operations module if significant efforts were made to improve the usability API and move away from a 1:1 mapping of the CLI interface to the Operations API.

mheath avatar May 01 '23 20:05 mheath

@mheath @dmikusa I would like to revive the discussion as mentioned for large CF setups, the usage of v2 API over v3 has performance and thus cost consequences. Are you ok with a pragmatic approach like migrating the Operations classes to v3 in a case by case manner. In classes where we have simple mapping between the apis for example we can do the switch for v3, which has the added benefit of low adoption cost for consumers. In cases, where mapping is not straight-forward or impact usability, we can proceed to creating a more meaningful/usable Operations class.

For us in SAP the topic is important and we are willing to put effort in moving it forward. Let me know what you think. Meanwhile I'll start few PRs, so we have tangible discussion points.

radoslav-tomov avatar Jul 04 '23 07:07 radoslav-tomov

I am very open to migrating Operations classes to V3 in a case by case manner. What you propose is very reasonable. I don't work on CFJC full time but I'll do my best to support your efforts.

mheath avatar Jul 20 '23 19:07 mheath