search

tcplugins / tcWebHooks

WebHooks plugin for Teamcity. Supports many build states and payload formats.
https://netwolfuk.wordpress.com/category/teamcity/tcplugins/tcwebhooks/
157 stars 30 forks source link

Deleting used build config results in non-editable webhook #242

Closed matt-richardson closed 5 months ago

matt-richardson commented 6 months ago

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

netwolfuk commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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.

matt-richardson commented 6 months ago

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:

netwolfuk commented 6 months ago

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

matt-richardson commented 5 months ago

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.

netwolfuk commented 5 months ago

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.

matt-richardson commented 5 months ago

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.

netwolfuk commented 5 months ago

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 commented 5 months ago

The build has run.

oooh, it's build 500. Exciting.

netwolfuk commented 5 months ago

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]
matt-richardson commented 5 months ago

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

netwolfuk commented 5 months ago

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 commented 5 months ago

Hi @matt-richardson

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

matt-richardson commented 5 months ago

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.)

netwolfuk commented 5 months ago

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.

matt-richardson commented 5 months ago

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.

netwolfuk commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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