Skip to content
This repository was archived by the owner on Dec 20, 2025. It is now read-only.

fix(web): fix the mismatches in the return types of APIs in gate and orca#1883

Merged
mergify[bot] merged 5 commits intospinnaker:masterfrom
kirangodishala:fix-delete-pipline-execution-error
Mar 1, 2025
Merged

fix(web): fix the mismatches in the return types of APIs in gate and orca#1883
mergify[bot] merged 5 commits intospinnaker:masterfrom
kirangodishala:fix-delete-pipline-execution-error

Conversation

@kirangodishala
Copy link
Contributor

  • While some of the Orca's API have void return types, Gate has Map return type for the corresponding APIs causing the following error :
Failed to process response body: No content to map due to end-of-input\n at [Source: (okhttps.ResponseBody$BomAwareReader); line:1, column:0)
  • Retrofit1 ignored the mismatches but retrofit2 has stricter response handling.
  • This PR adds a test to demonstrate the issue and addressed the mismatches.

…es of gate and orca when delete pipeline execution api is invoked
@kirangodishala kirangodishala requested a review from dbyron-sf March 1, 2025 14:29

@Autowired ObjectMapper objectMapper;

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this in these tests?

Copy link
Contributor Author

@kirangodishala kirangodishala Mar 1, 2025

Choose a reason for hiding this comment

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

Do you mean autowiring of ObjectMapper? It's not. I will update. But if you are asking whether ObjectMapper is needed at all, there is a usage in the test to create the response body string.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was asking about the FilterRegistrationBean

Copy link
Contributor

Choose a reason for hiding this comment

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

And it turns out that it is needed to silence warnings about e.g. missing X-SPINNAKER-USER, X-SPINNAKER-ACCOUNTS warnings...

Copy link
Contributor

Choose a reason for hiding this comment

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

I pushed some commits to silence those warnings, and some others.

@kirangodishala kirangodishala force-pushed the fix-delete-pipline-execution-error branch from 989ce8c to b529d7f Compare March 1, 2025 17:26
kirangodishala and others added 4 commits March 1, 2025 11:21
…orca for delete, resume, cancel and pause operations of a pipeline.
…ders

to requests in PipelineServiceTest to silence warnings like:

2025-03-01 11:28:27.487  WARN 98848 --- [    Test worker] c.n.s.okhttp.OkHttp3MetricsInterceptor   : [] Request GET:http://localhost:52447/pipelines/my-pipeline-execution-id is missing [X-SPINNAKER-USER, X-SPINNAKER-ACCOUNTS] authentication headers and will be treated as anonymous.
Request from: com.netflix.spinnaker.okhttp.MetricsInterceptor.doIntercept(MetricsInterceptor.java:82)
	at com.netflix.spinnaker.okhttp.OkHttp3MetricsInterceptor.intercept(OkHttp3MetricsInterceptor.java:36)
	at com.netflix.spinnaker.okhttp.SpinnakerRequestHeaderInterceptor.intercept(SpinnakerRequestHeaderInterceptor.java:64)
	at com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory$ExecutorCallbackCall.execute(ErrorHandlingExecutorCallAdapterFactory.java:150)
	at com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall.executeCall(Retrofit2SyncCall.java:47)
	at com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall.execute(Retrofit2SyncCall.java:34)
	at com.netflix.spinnaker.gate.services.PipelineService.getPipeline(PipelineService.groovy:170)
	at com.netflix.spinnaker.gate.services.PipelineService$_setApplicationForExecution_closure2.doCall(PipelineService.groovy:222)
	at com.netflix.spinnaker.kork.core.RetrySupport.retry(RetrySupport.java:34)
	at com.netflix.spinnaker.kork.core.RetrySupport.retry(RetrySupport.java:27)
	at com.netflix.spinnaker.gate.services.PipelineService.setApplicationForExecution(PipelineService.groovy:222)
	at com.netflix.spinnaker.gate.services.PipelineService.deletePipeline(PipelineService.groovy:189)
	at com.netflix.spinnaker.gate.controllers.PipelineController.deletePipeline(PipelineController.groovy:279)
	at com.netflix.spinnaker.gate.service.PipelineServiceTest.invokeDeletePipelineExecution(PipelineServiceTest.java:135)
@dbyron-sf dbyron-sf force-pushed the fix-delete-pipline-execution-error branch from b529d7f to 073d67a Compare March 1, 2025 19:30
@dbyron-sf dbyron-sf added the ready to merge Approved and ready for merge label Mar 1, 2025
@mergify mergify bot added the auto merged label Mar 1, 2025
@mergify mergify bot merged commit 15465c9 into spinnaker:master Mar 1, 2025
5 checks passed
@dbyron-sf
Copy link
Contributor

@Mergifyio backport release-1.37.x

@mergify
Copy link
Contributor

mergify bot commented Mar 1, 2025

backport release-1.37.x

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Mar 1, 2025
…orca (#1883)

* test(web): add a test to demonstrate the mismatch in api response types of gate and orca when delete pipeline execution api is invoked

* fix(web): fix the mismatches in the return types of apis in gate and orca for delete, resume, cancel and pause operations of a pipeline.

* refactor(web/test): mock DefaultProviderLookupService in PipelineServiceTest

to remove errors from test output

* refactor(web/test): remove request id header from PipelineServiceTest

since it's not needed

* refactor(web/test): add X-SPINNAKER-USER and X-SPINNAKER-ACCOUNTS headers

to requests in PipelineServiceTest to silence warnings like:

2025-03-01 11:28:27.487  WARN 98848 --- [    Test worker] c.n.s.okhttp.OkHttp3MetricsInterceptor   : [] Request GET:http://localhost:52447/pipelines/my-pipeline-execution-id is missing [X-SPINNAKER-USER, X-SPINNAKER-ACCOUNTS] authentication headers and will be treated as anonymous.
Request from: com.netflix.spinnaker.okhttp.MetricsInterceptor.doIntercept(MetricsInterceptor.java:82)
	at com.netflix.spinnaker.okhttp.OkHttp3MetricsInterceptor.intercept(OkHttp3MetricsInterceptor.java:36)
	at com.netflix.spinnaker.okhttp.SpinnakerRequestHeaderInterceptor.intercept(SpinnakerRequestHeaderInterceptor.java:64)
	at com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory$ExecutorCallbackCall.execute(ErrorHandlingExecutorCallAdapterFactory.java:150)
	at com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall.executeCall(Retrofit2SyncCall.java:47)
	at com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall.execute(Retrofit2SyncCall.java:34)
	at com.netflix.spinnaker.gate.services.PipelineService.getPipeline(PipelineService.groovy:170)
	at com.netflix.spinnaker.gate.services.PipelineService$_setApplicationForExecution_closure2.doCall(PipelineService.groovy:222)
	at com.netflix.spinnaker.kork.core.RetrySupport.retry(RetrySupport.java:34)
	at com.netflix.spinnaker.kork.core.RetrySupport.retry(RetrySupport.java:27)
	at com.netflix.spinnaker.gate.services.PipelineService.setApplicationForExecution(PipelineService.groovy:222)
	at com.netflix.spinnaker.gate.services.PipelineService.deletePipeline(PipelineService.groovy:189)
	at com.netflix.spinnaker.gate.controllers.PipelineController.deletePipeline(PipelineController.groovy:279)
	at com.netflix.spinnaker.gate.service.PipelineServiceTest.invokeDeletePipelineExecution(PipelineServiceTest.java:135)

---------

Co-authored-by: David Byron <[email protected]>
(cherry picked from commit 15465c9)
mergify bot added a commit that referenced this pull request Mar 1, 2025
…orca (#1883) (#1884)

* test(web): add a test to demonstrate the mismatch in api response types of gate and orca when delete pipeline execution api is invoked

* fix(web): fix the mismatches in the return types of apis in gate and orca for delete, resume, cancel and pause operations of a pipeline.

* refactor(web/test): mock DefaultProviderLookupService in PipelineServiceTest

to remove errors from test output

* refactor(web/test): remove request id header from PipelineServiceTest

since it's not needed

* refactor(web/test): add X-SPINNAKER-USER and X-SPINNAKER-ACCOUNTS headers

to requests in PipelineServiceTest to silence warnings like:

2025-03-01 11:28:27.487  WARN 98848 --- [    Test worker] c.n.s.okhttp.OkHttp3MetricsInterceptor   : [] Request GET:http://localhost:52447/pipelines/my-pipeline-execution-id is missing [X-SPINNAKER-USER, X-SPINNAKER-ACCOUNTS] authentication headers and will be treated as anonymous.
Request from: com.netflix.spinnaker.okhttp.MetricsInterceptor.doIntercept(MetricsInterceptor.java:82)
	at com.netflix.spinnaker.okhttp.OkHttp3MetricsInterceptor.intercept(OkHttp3MetricsInterceptor.java:36)
	at com.netflix.spinnaker.okhttp.SpinnakerRequestHeaderInterceptor.intercept(SpinnakerRequestHeaderInterceptor.java:64)
	at com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory$ExecutorCallbackCall.execute(ErrorHandlingExecutorCallAdapterFactory.java:150)
	at com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall.executeCall(Retrofit2SyncCall.java:47)
	at com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall.execute(Retrofit2SyncCall.java:34)
	at com.netflix.spinnaker.gate.services.PipelineService.getPipeline(PipelineService.groovy:170)
	at com.netflix.spinnaker.gate.services.PipelineService$_setApplicationForExecution_closure2.doCall(PipelineService.groovy:222)
	at com.netflix.spinnaker.kork.core.RetrySupport.retry(RetrySupport.java:34)
	at com.netflix.spinnaker.kork.core.RetrySupport.retry(RetrySupport.java:27)
	at com.netflix.spinnaker.gate.services.PipelineService.setApplicationForExecution(PipelineService.groovy:222)
	at com.netflix.spinnaker.gate.services.PipelineService.deletePipeline(PipelineService.groovy:189)
	at com.netflix.spinnaker.gate.controllers.PipelineController.deletePipeline(PipelineController.groovy:279)
	at com.netflix.spinnaker.gate.service.PipelineServiceTest.invokeDeletePipelineExecution(PipelineServiceTest.java:135)

---------

Co-authored-by: David Byron <[email protected]>
(cherry picked from commit 15465c9)

Co-authored-by: Kiran Godishala <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants