Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tenant ID cannot be empty string error #125

Open
hferentschik opened this issue Feb 6, 2018 · 20 comments
Open

Tenant ID cannot be empty string error #125

hferentschik opened this issue Feb 6, 2018 · 20 comments

Comments

@hferentschik
Copy link
Contributor

I created a new codebase on prod-preview and when looking at the first delivery of the newly generated webhook I see:

{"Errors":[{"code":"500","detail":"Tenant ID cannot be empty string"}]}

When I then re-deliver the webhook request, I get a status of 200.

@hferentschik
Copy link
Contributor Author

In another attempt, I got a pipeline created, but the corresponding codebase did not appear in the UI. In this case, all re-deliveries failed. I guess we do the lookup of the user based on the codebase. I then added the codebase explicitly and delivery of the hook worked.

@hferentschik
Copy link
Contributor Author

@vpavlin any ideas. Is this something you have seen before?

@hferentschik
Copy link
Contributor Author

hferentschik commented Feb 7, 2018

@gastaldi - how does the wizard code work? In which order do you create the repository, add the webhook and do the "registration" of the repository with the openshift.io platform.

I am asking since I am wondering whether you create the repository and webhook prior to making a call to our backend. This would explain what I am seeing. The moment you set the webhook, GitHub will send a test request. On the proxy side, we need to identify this webhook request and match it to the right actual Jenkins instance. We do this by looking at the webhook payload and matching the repository URL. If at the time the request comes in, we don't know about the repository yet, we cannot match/route the request.

If that's the case, the workflow in the wizard needs probably adjusting.

@gastaldi
Copy link
Contributor

gastaldi commented Feb 7, 2018

@hferentschik by reading the code, the order is as follows:

  1. Github repository is created
  2. Code is pushed to the repository
  3. BuildConfig is created in Openshift
  4. Jenkins job is created
  5. Build is triggered
  6. Webhoook is created

@hferentschik
Copy link
Contributor Author

@gastaldi interesting. I guess we need to dig a bit deeper. So how does the WIT service determine the owner of a repository when using the endpoint /api/search/codebases? @vpavlin Do you know?

@aslakknutsen
Copy link

@hferentschik repo_url to codebase/url db lookup ?

@hferentschik
Copy link
Contributor Author

Right, so something is written to a database somewhere. Looking at @gastaldi steps I am wondering where this part is happening. I mean the part which records a given repo as a codebase for a given user. It does not seem to be part of it.

@vpavlin
Copy link
Member

vpavlin commented Feb 7, 2018

I would not be surprised by that - OSIO only calls Launcher as a service, so it probably returns a repo URL and tenant service records it?

Is the problem that the creation of the webhook triggers it? Which fails in Proxy because Tenant does not have entry for that codebase?

@aslakknutsen
Copy link

@hferentschik That's a UI 'feature' as far as I'm aware. I believe the Launcher returns a Repo url from the Wizard end call, which the UI then creates a Codebase for.

@hferentschik
Copy link
Contributor Author

I would not be surprised by that - OSIO only calls Launcher as a service, so it probably returns a repo URL and tenant service records it?

So why would the Launcher not also record the codebase? What's the benefit of doing it this way?

Is the problem that the creation of the webhook triggers it?

Yes, afaiu creating the web hook also triggers a delivery.

Which fails in Proxy because Tenant does not have entry for that codebase?

I think so. The web hook is delivered, but we cannot lookup the owner of the request since apparently the codebase does not get registered until later.

@vpavlin
Copy link
Member

vpavlin commented Feb 7, 2018

So why would the Launcher not also record the codebase? What's the benefit of doing it this way?

As I said - Launcher does not know anything about OSIO, it does not contact any OSIO services, it's only called as a backend API by UI - so it cannot really record the codebase because it does not have any notion of where and what to record

@hferentschik
Copy link
Contributor Author

That's a UI 'feature' as far as I'm aware. I believe the Launcher returns a Repo url from the Wizard end call, which the UI then creates a Codebase for

And what's the benefit? Would it not make sense for the launcher to "record" the codebase the moment it creates the repository?

I guess it is not a big issue and it does not effect us directly, still it is not very nice and potentially confusing for a user to see failing web hook deliveries.

@vpavlin
Copy link
Member

vpavlin commented Feb 7, 2018

And what's the benefit?

That is a question for @gastaldi to answer:) Launcher originated as a completely separate project and was later used as a part of OSIO - not sure what the future plans are on integration/overlap.

@aslakknutsen
Copy link

aslakknutsen commented Feb 7, 2018

@hferentschik I'm not saying there is a benefit, just telling you how it works. All this stuff should happen in the backend. Tech Dep. Not to say it should be in Launcher.

@gastaldi
Copy link
Contributor

gastaldi commented Feb 7, 2018

Since we are rewriting the whole OSIO integration in Launcher perhaps that's something that we could consider in the future. We really want to avoid any major changes to the legacy code right now.

@hferentschik
Copy link
Contributor Author

I'm not saying there is a benefit, just telling you how it works.

Sure. And I am trying to figure out whether there was/is a driving reason for the chosen design/implementation :-)

All this stuff should happen in the backend.

Sounds reasonable. I guess there are several ways to to this. The current way seems less favourable.

@hferentschik
Copy link
Contributor Author

Since we are rewriting the whole OSIO integration in Launcher perhaps that's something that we could consider in the future.

+1 for taking this into consideration. As @aslakknutsen is saying, it either should happen in the backend or you design your services a bit differently. Potentially more granular.

We really want to avoid any major changes to the legacy code right now.

Is this really a viable option? Wherever one looks, one gets to hear that this or that will be fixed by the re-design or re-write of component x, y and z. Is this really realistic to think we can rewrite the whole thing? Is it not really a case of trying to work with what we have and gradually make things better?

@gastaldi
Copy link
Contributor

gastaldi commented Feb 7, 2018

The refactor is already happening here: https://github.com/fabric8-launcher/launcher-backend/tree/refactor

As I mentioned in the past issues, the legacy is unmaintainable and untestable, that's the main reason we want to get rid of it

@hferentschik
Copy link
Contributor Author

So are we talking about refactoring or effectively a re-write? In the latter case, are there plans to change the API and how this fits together? It seems the current flow is less than optimal.

@gastaldi
Copy link
Contributor

gastaldi commented Feb 7, 2018

It's a rewrite. I'm basing my work in the legacy code right now, but we can definitely talk about changing it before that happens.

My point is that it doesn't make sense to invest time in the legacy code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants