After realizing the issue stems from different paths during build, I thought it would be better to use consistent names for the modules that don’t rely on the relative path to the module. To address that I opened:
which simply strips all the ../../../node_modules/ part of the module ID, leaving just the module’s path inside the node_modules folder. This should allow packaging process to run from any path while producing consistent results.
The plugin works with any kind of relative paths, and it fixes all of Katello, REX and Ansible!
I am fairly confident that nightlies will be fixed tonight. If they pass and we can check Job Invocations, Ansible reports, and RH subscriptions, we can conclude this thread (thanks to everyone involved!).
Let’s keep in mind we still have other packaging matters to worry about after this:
debs unable to take webpack assets from plugins (we haven’t even tried AFAIK)
we should make sure the machine where we build our packages does not matter. e.g: if now, we have a Fedora machine in koji which builds the ‘katello’ package (including webpack assets), and a Centos machine in Koji which builds the ‘foreman’ package, I think it would not work.
SimpleNamedModule is in place and seems to have solved the issue for most modules, now for most modules katello manages to find the correct module in vendor.js.
However, two modules katello expects to be in vendor.js aren’t there - warning/warning.js and invariant/browser.js. As far as I can tell, these are being required from react-router.
This might be due to the fact these modules are present in the core foreman node_modules, but possibly nothing in foreman uses them so they are tree-shaken out of vendor.js, while when katello builds they are used and thus remain in vendor.js. This is just a theory at this point though, not anything substantiated yet.
Looking through the Katello staging RPM currently, I see the following in the code for example:
"../../../../../BUILDROOT/tfm-rubygem-katello-3.8.0-.201806261113gitabbff481.nightly.el7.noarch/opt/theforeman/tfm/root/usr/share/gems/gems/katello-3.8.0/webpack/containers/Application/overrides.scss": function(e, t) {},
"../../../../../BUILDROOT/tfm-rubygem-katello-3.8.0-.201806261113gitabbff481.nightly.el7.noarch/opt/theforeman/tfm/root/usr/share/gems/gems/katello-3.8.0/webpack/index.js": function(e, t, o) {
"use strict";
var n = s(o("./webpack/assets/javascripts/react_app/components/componentRegistry.js"))
, r = o("./webpack/assets/javascripts/react_app/common/MountingService.js")
, a = s(o("../../../../../BUILDROOT/tfm-rubygem-katello-3.8.0-.201806261113gitabbff481.nightly.el7.noarch/opt/theforeman/tfm/root/usr/share/gems/gems/katello-3.8.0/webpack/containers/Application/index.js"));
function s(e) {
return e && e.__esModule ? e : {
default: e
}
}
o("../../../../../BUILDROOT/tfm-rubygem-katello-3.8.0-.201806261113gitabbff481.nightly.el7.noarch/opt/theforeman/tfm/root/usr/share/gems/gems/katello-3.8.0/webpack/redux/index.js"),
n.default.register({
name: "xui_katello",
type: a.default
}),
(0,
r.mount)("xui_katello", "#reactRoot")
},
"../../../../../BUILDROOT/tfm-rubygem-katello-3.8.0-.201806261113gitabbff481.nightly.el7.noarch/opt/theforeman/tfm/root/usr/share/gems/gems/katello-3.8.0/webpack/move_to_foreman/Settings/SettingsActions.js": function(e, t, o) {
"use strict";
Object.defineProperty(t, "__esModule", {
value: !0
}),
t.loadSetting = void 0;
var n = o("../../../../../BUILDROOT/tfm-rubygem-katello-3.8.0-.201806261113gitabbff481.nightly.el7.noarch/opt/theforeman/tfm/root/usr/share/gems/gems/katello-3.8.0/webpack/services/api/index.js")
, r = o("../../../../../BUILDROOT/tfm-rubygem-katello-3.8.0-.201806261113gitabbff481.nightly.el7.noarch/opt/theforeman/tfm/root/usr/share/gems/gems/katello-3.8.0/webpack/move_to_foreman/Settings/SettingsConstants.js")
, a = t.loadSetting = function(e) {
return function(t) {
return t({
type: r.GET_SETTING_REQUEST
}),
n.foremanApi.get("/settings/" + e).then(function(e) {
var o = e.data;
t({
type: r.GET_SETTING_SUCCESS,
response: o
})
}).catch(function(e) {
t({
type: r.GET_SETTING_FAILURE,
result: e
})
})
}
}
;
t.default = a
},
"../../../../../BUILDROOT/tfm-rubygem-katello-3.8.0-.201806261113gitabbff481.nightly.el7.noarch/opt/theforeman/tfm/root/usr/share/gems/gems/katello-3.8.0/webpack/move_to_foreman/Settings/SettingsConstants.js": function(e, t, o) {
"use strict";
Note the BUILDROOT pathing artifact which from the RPM build which seems wrong and bad to have it resolving that way and possibly causing errors? I wonder if we need to make those also check for pluginName when generating and cut off the path so its relative to the plugin?
I think as long as its katello calling katello code the buildroot paths are fine - remember, these are just module identifiers, not real paths being used. The paths starting with ./webpack are calls to foreman core modules, which are also identified in the same way in foreman’s bundle.js so we’re good. I think the only case where we may see issues here is if one plugin tries to call another plugin’s js code, which I would argue is a Bad Idea anyways.
I think as long as its katello calling katello code the buildroot paths
are fine - remember, these are just module identifiers, not real paths
being used. The paths starting with ./webpack are calls to foreman core
modules, which are also identified in the same way in foreman’s bundle.js
so we’re good. I think the only case where we may see issues here is if one
plugin tries to call another plugin’s js code, which I would argue is a Bad
Idea[image: ] anyways.
unless, a plugin such as foreman tasks add generic components to handle
tasks…
in that case I would argue the plugin should expose an api allowing other plugins to interact with it, rather then need to include components in it - especially if some day in the future it may even be a completely separate container or something.
With some help from @sharvit, we managed to figure out why katello is expecting these two modules as top-level modules in vendor.js:
This PR added packages for the history, warning, and invariant npm modules, leading them to be installed as top-level modules in /usr/lib/node_modules when building katello. Since webpack finds them there, and some other modules are also using them, it adds them to the vendor.js file that is generated during the katello build (and subsequently discarded), and makes all calls to them from katello.js point to the module in vendor.js, instead of including the nested version from inside the react-router module into katello.js.
Next step, I will open a PR to packaging removing these modules from the packaging (as they are and should be packaged as bundled dependencies of react-router, not separately)
anyways.