tcWebHooks icon indicating copy to clipboard operation
tcWebHooks copied to clipboard

Deleting used build config results in non-editable webhook

Open matt-richardson opened this issue 1 year ago • 9 comments

Expected Behavior

A webhook configured to point to deleted build config should be able to be loaded.

Current Behavior

We get an error image

Steps to Reproduce (for bugs)

  1. Create a teamcity project Project1
  2. Create 2 build configs within that project BuildConfigA and BuildConfigB
  3. Create a webhook in Project1, that only triggers on BuildConfigB
    1. Viewing the project-config.xml shows that the id is stored in the build-types image
  4. Delete BuildConfigB
  5. Go to the root webhook page (/admin/admin.html?item=tcWebHooks)
  6. Click on the "There a n WebHooks configured in this TeamCity Server" link
  7. Click on Edit project Webhooks for Project1
  8. Click on edit for the webhook created in step 3 -> get failure popup An unexpected error occured. Please see your browser's javascript console.

Console log has:

<div class="errorPage">
  <h1 class="errorTitle">Unexpected Error</h1>
  <p>
    This was not supposed to happen.
    Please provide the error details to your TeamCity server maintainer.<br/>
    If you maintain this TeamCity installation and the request seems legit please
    <a href="https://www.jetbrains.com/help/teamcity/feedback.html">report</a> this error to JetBrains.
 </p>
  <div class="errorMessage">
    Error: webhook.teamcity.exception.NonExistantProjectException: No build type found with matching internal Id:bt62
  </div>
  <br/>
  <div>
    Server time: 2024-04-19 07:07:02
  </div>
<div>
  TeamCity: 2024.03 (build 156166)
</div>

with an stack trace of:

Trace: webhook.teamcity.exception.NonExistantProjectException: No build type found with matching internal Id:bt62
    at webhook.teamcity.ProjectAndBuildTypeResolverImpl.getExternalBuildTypeId(ProjectAndBuildTypeResolverImpl.java:51)
    at webhook.teamcity.ProjectAndBuildTypeResolverImpl.getExternalBuildTypeIds(ProjectAndBuildTypeResolverImpl.java:71)
    at webhook.teamcity.json.WebHookConfigurationJson.fromWebHookConfig(WebHookConfigurationJson.java:83)
    at webhook.teamcity.extension.WebHookEditController.doHandle(WebHookEditController.java:74)
    at jetbrains.buildServer.controllers.BaseController.handleRequestInternal(BaseController.java:113)
    at org.springframework.web.servlet.mvc.AbstractController.handleRequest(AbstractController.java:177)
    at jetbrains.buildServer.controllers.BaseController.handleRequest(BaseController.java:92)
    at org.springframework.web.servlet.mvc.SimpleControllerHandlerAdapter.handle(SimpleControllerHandlerAdapter.java:51)
    at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1072)
    at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:965)
    at jetbrains.buildServer.maintenance.WebDispatcherServlet.doService(WebDispatcherServlet.java:17)
    at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)
    at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:529)
    at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:623)
    at jetbrains.buildServer.maintenance.TeamCityDispatcherServlet.processedByMainServlet(TeamCityDispatcherServlet.java:20)
    at jetbrains.buildServer.maintenance.TeamCityDispatcherServlet.service(TeamCityDispatcherServlet.java:76)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:210)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:154)
    at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:51)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:179)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:154)
    at jetbrains.buildServer.https.HttpsRedirectFilter.doFilter(HttpsRedirectFilter.java:32)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:179)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:154)
    at jetbrains.buildServer.web.DisableSessionIdFromUrlFilter.doFilter(DisableSessionIdFromUrlFilter.java:8)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:179)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:154)
    at jetbrains.buildServer.web.UserIdProviderFilter.doFilter(UserIdProviderFilter.java:8)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:179)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:154)
    at jetbrains.buildServer.web.BannedIPsFilter.doFilter(BannedIPsFilter.java:5)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:179)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:154)
    at jetbrains.buildServer.web.NodeInfoHeaderFilter.doFilter(NodeInfoHeaderFilter.java:9)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:179)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:154)
    at org.springframework.web.filter.CompositeFilter$VirtualFilterChain.doFilter(CompositeFilter.java:108)
    at jetbrains.buildServer.diagnostic.web.DiagnosticFilter.doFilter(DiagnosticFilter.java:8)
    at org.springframework.web.filter.CompositeFilter$VirtualFilterChain.doFilter(CompositeFilter.java:113)
    at jetbrains.buildServer.web.DependencyParametersCalculationContextFilter.doFilter(DependencyParametersCalculationContextFilter.java:2)
    at org.springframework.web.filter.CompositeFilter$VirtualFilterChain.doFilter(CompositeFilter.java:113)
    at jetbrains.buildServer.diagnostic.web.HttpRequestsDurationMetricsReporter.doFilter(HttpRequestsDurationMetricsReporter.java:5)
    at org.springframework.web.filter.CompositeFilter$VirtualFilterChain.doFilter(CompositeFilter.java:113)
    at jetbrains.buildServer.web.HttpSecurityHeadersFilter.doFilter(HttpSecurityHeadersFilter.java:29)
    at org.springframework.web.filter.CompositeFilter$VirtualFilterChain.doFilter(CompositeFilter.java:113)
    at jetbrains.buildServer.controllers.filters.InvalidPathPattermsFilter.doFilter(InvalidPathPattermsFilter.java:14)
    at org.springframework.web.filter.CompositeFilter$VirtualFilterChain.doFilter(CompositeFilter.java:113)
    at jetbrains.buildServer.controllers.filters.DisableSessionCookieTokenAuthFilter.doFilter(DisableSessionCookieTokenAuthFilter.java:4)
    at org.springframework.web.filter.CompositeFilter$VirtualFilterChain.doFilter(CompositeFilter.java:113)
    at jetbrains.buildServer.controllers.filters.ProxyDetailsFilter.doFilter(ProxyDetailsFilter.java:9)
    at org.springframework.web.filter.CompositeFilter$VirtualFilterChain.doFilter(CompositeFilter.java:113)
    at jetbrains.buildServer.controllers.filters.ClearSecurityContextFilter.doFilter(ClearSecurityContextFilter.java:24)
    at org.springframework.web.filter.CompositeFilter$VirtualFilterChain.doFilter(CompositeFilter.java:113)
    at org.springframework.web.filter.CompositeFilter.doFilter(CompositeFilter.java:74)
    at jetbrains.buildServer.web.DelegatingFilter.doFilter(DelegatingFilter.java:74)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:179)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:154)
    at jetbrains.buildServer.web.ResponseFragmentFilter.doFilter(ResponseFragmentFilter.java:27)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:179)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:154)
    at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:168)
    at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:90)
    at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:481)
    at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:130)
    at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:93)
    at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74)
    at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:346)
    at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:390)
    at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:63)
    at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:928)
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1786)
    at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:52)
    at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191)
    at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659)
    at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:63)
    at java.base/java.lang.Thread.run(Thread.java:840)

Your Environment

  • tcWebHooks Version:
  • TeamCity Version: 2024.03
  • TeamCity server Operating System: Mac
  • Are you using a WebHook Template?: No

matt-richardson avatar Apr 18 '24 21:04 matt-richardson

I suspect the same will happen when editing a webhook via the REST api (which is how it works on a project edit page)

netwolfuk avatar Apr 18 '24 21:04 netwolfuk

I have a fix that looks like it works. I need to get the coverage up on this code, but will try to push later today or tomorrow.

netwolfuk avatar Apr 24 '24 06:04 netwolfuk

BTW, I found another bug. Copying a project that contains webhooks with specific builds configured does not update those WebHooks with the new buildType ids in the new project.

netwolfuk avatar Apr 24 '24 21:04 netwolfuk

So it looks like copying a project fires off the projectCreated event. There is no "projectCopied" event that I can find.

If a project is new, then it won't have webhooks configured. However, if it has some configured AND any webhook specifically references a buildId AND that buildId does not correspond to a build in the newly created project, I can assume it was a copy, not a create.

Since I have the old buildId, I can then find that old build, get the name, find one in the new project with the same name, and swap the buildId in my webhook configs.

edit: Name might not be unique in a project, so using BuildTypeExternalId and parsing it to match the new project (eg, .endsWith() might be safer.

netwolfuk avatar Apr 25 '24 01:04 netwolfuk

BTW, I found another bug. Copying a project that contains webhooks with specific builds configured does not update those WebHooks with the new buildType ids in the new project.

it's the gift that keeps on giving :wry-smile:

matt-richardson avatar Apr 25 '24 02:04 matt-richardson

New commit pushed. It will build soon. No coverage improvements yet, but now has support for deleted builds.

netwolfuk avatar Apr 25 '24 09:04 netwolfuk

Finally got a chance to do some testing on this...

On first try I got a 404 trying to edit the webhook, which I couldn't repro, which was annoying. I couldn't find anything in the logs though, double annoying.

I cleared out the logs (to get rid of the noise), then restarted the server, then tried again, noting down what I did as I went:

  1. create project in the root called "Testing tcWebhooks Issue 242
  2. create subproject "Sub Project 1"
  3. create build config "Build Config 1" in "Sub Project 1"
  4. create build config "Build Config 2" in "Sub Project 1"
  5. create a webhook in "Sub Project 1", but disable it for "Build Config 2"
  6. delete "Build Config 2"
  7. edit the webhook - shows correctly, saves correctly
  8. create build config "Build Config 3" in "Sub Project 1"
  9. edit the webhook - shows correctly
  10. copy "Sub Project 1" to "Sub Project 2"
  11. go edit the webhook - shows correctly
  12. enable the webhook in "Sub Project 2" for "Build Config 2", saves correctly
  13. copy "Sub Project 2" to "Sub Project 3"
  14. click on webhooks, webhook does not appear on Sub Project 3
  15. refresh after ~15 seconds, webhook appears
  16. edit the webhook, no builds selected

I think 14 is related to the 10 second delay (if that's still in there), but not sure what's the go with 16.

I reckon we're at "usable", but there's still some gremlins in there.

matt-richardson avatar May 05 '24 01:05 matt-richardson

I found another event that might help. projects.ProjectEventsNotifier - Attempting to publish event about projectExtId TcPlugins3. Cause projectPersisted This might be useful to speed up the 10 second window (14). At this point, I don't know why (moved, created, etc).

With regards to 16, I have not written that part yet. The last couple of weeks have been crazy busy.

netwolfuk avatar May 06 '24 08:05 netwolfuk

With regards to 16, I have not written that part yet. The last couple of weeks have been crazy busy.

I know that feeling all too well :) No rush, no stress - appreciate all the work you're putting in here.

matt-richardson avatar May 08 '24 01:05 matt-richardson

I've pushed an update with the following changes:

  1. Uses a different event projectPersisted. From what I can tell, this fires after persist and reload.
  2. projectPersisted triggers a refresh after just 1 second, instead of 10 seconds like before.
  3. Old BuildTypeIds are mapped to new BuildTypeIds if (and only if) the WebHook's unique id clashes. Then a re-hash of the webhook uniqueId is undertaken, and the buildTypeIds are attempted to be mapped.
  4. Updated Jersey (on the legacy REST API) to 1.19 (from 1.16). This updates ASM to a Java8 version and allows me to use lambdas and java streams (whoop whoop).

The algorithm for remapping is fairly brittle. It assumes the following:

  1. BuildType External Ids are in the format ProjectExternalId_BuildTypeId
  2. That new projects will have changed the ProjectExternalId part, but not the BuildTypeId
  3. Typically have underscore separators , but don't need to.
  4. Case is ignored when mapping.

In code, the unit test looks like this...

assertTrue(WebHookSettingsManagerImpl.fuzzyNameMatcher("Project1", "Project1_Build01", "Project02", "Project02_Build01"));
assertTrue(WebHookSettingsManagerImpl.fuzzyNameMatcher("project1", "Project1_Build01", "Project02", "Project02_Build01"));
assertTrue(WebHookSettingsManagerImpl.fuzzyNameMatcher("Project1", "Project1Build01", "Project02", "Project02Build01"));

I have done zero testing on the legacy Rest API. My main TeamCity VM is off at the moment. I have only tested on my developer workstation, which has a newer TeamCity installed.

netwolfuk avatar May 13 '24 10:05 netwolfuk

The build has run.

oooh, it's build 500. Exciting.

netwolfuk avatar May 13 '24 10:05 netwolfuk

I forgot to say that the logs look like this...

INFO -   jetbrains.buildServer.SERVER - WebHookSettingsManagerImpl :: WebHook uniqueId selected as candidate for updating to avoid conflict with existing webhooks. projectId: 'project161', oldUniqueId: 'id_465810593', newUniqueId: 'id_987758698'
INFO -   jetbrains.buildServer.SERVER - WebHookSettingsManagerImpl :: Remapping buildTypdIds for WebHook. projectId: 'project161', newUniqueId: 'id_987758698', oldIds: [bt164], newIds: [bt172]

netwolfuk avatar May 13 '24 10:05 netwolfuk

Checking... This branch includes the fixes for #238 as well, yeah?

matt-richardson avatar May 14 '24 07:05 matt-richardson

Checking... This branch includes the fixes for https://github.com/tcplugins/tcWebHooks/issues/238 as well, yeah?

Yes, it's supposed to, unless I have introduced a regression

netwolfuk avatar May 14 '24 07:05 netwolfuk

Hi @matt-richardson

Just checking in to see if you have any questions on this. Or need any help testing it.

netwolfuk avatar May 20 '24 05:05 netwolfuk

Sorry for the delay - finally found some time to test it out - cant fault it. Tried a bunch of combinations around copying projects and filtering / deleting specific build configs Nice one!

(The only thing I did notice was that if you set up 2 build configs, filter to 1 of them, then delete the one you have, it stays in the config. UI still all works though - just ignores it.)

matt-richardson avatar May 22 '24 06:05 matt-richardson

Thank you so much for your time. I really appreciate it.

If you edit the webhook after the build is deleted, does it exclude that BT id when writing the webhook config? I suspect it does. As long as editing works, I'm not too concerned about the delete use case, just in case it is undeleted again.

netwolfuk avatar May 22 '24 06:05 netwolfuk

If you edit the webhook after the build is deleted, does it exclude that BT id when writing the webhook config? I suspect it does.

Yep, it removes it from the config when you hit sav.

Also realised that before you hit edit, the UI shows the correct 0 builds for "Enabled Builds":

image

Thank you so much for your time. I really appreciate it.

Hah! I'm doing the easy part. You're doing the hard work here.

matt-richardson avatar May 22 '24 06:05 matt-richardson

Sounds good to me! I'll plan to release before the weekend. If it doesn't happen by then it will be June because.... life

netwolfuk avatar May 22 '24 07:05 netwolfuk

Thank you so much for your time. I really appreciate it.

Hah! I'm doing the easy part. You're doing the hard work here.

It's just really motivating to have someone engaged and interested in the outcome of a piece of work. I appreciate that.

netwolfuk avatar May 22 '24 07:05 netwolfuk

Released https://github.com/tcplugins/tcWebHooks/releases/tag/v2.0.1

netwolfuk avatar May 25 '24 08:05 netwolfuk