refactor: DER-encode the receptor hostname without asn1 - #926
Closed
blaipr wants to merge 89 commits into
Closed
Conversation
Still included in by ansible-runner
Adds the toolchain: a tsconfig checked under strict with noUncheckedIndexedAccess, the eslint rules for TypeScript files, and the api types generated from the platform's own OpenAPI schema. Converts the util, api, hooks and contexts layers. Untyped, exported from types/api, marks a value whose shape has not been described yet: it is any with a name, so the remaining debt is greppable rather than invisible.
Renames the components layer, gives each component an exported props interface, and annotates what the checker could not infer. The interfaces are read back out of each component: a prop is optional where the component defaults, guards or forwards it, and required where it calls or dereferences it itself. Includes the repository's lint rules over the converted files, which is what turned up several of the fixes that follow.
PatternFly 6 calls a checkbox's onChange with (event, checked); these seven read the first argument as the flag, which is what PatternFly 5 put there. The flag was therefore an event object and always true, so unticking a day pushed it onto the list a second time instead of removing it, and the checkbox stayed ticked because its state is derived from that list. Present since the initial import; the PatternFly 6 upgrade is what changed the argument order underneath it.
…ated cannotDisassociate holds one of the two predicates, so it is always truthy and the ternary below it always took the instances branch. A list of anything else was checked for node_type === 'control', which nothing but an instance has, so the permission message never appeared.
Types the lookup reducer, the render props the lists pass down, the schedule list, occurrences and detail, the code editor and the wizard props the role wizard hands around.
Three sites wrote x === ('a' || 'b') or includes('a' || 'b'), both of which
evaluate the or first and compare against 'a' alone:
- an ad hoc command with the shell module and no arguments was accepted,
though the step marks the field required for shell as well as command
- touching only module_args ran neither of that step's checks
- the notification Test button stayed enabled while a test was pending, so a
second test could be started on top of the first
All three date from the initial import.
Types the remaining component layer: the ad hoc wizard's steps under one shared shape, the credential steps, the label and multi selects, the launch entry points, the lists and their toolbars, the status maps, the form fields and the schedule editor. The components layer checks clean at this point.
Converts the management jobs, dashboard, topology and workflow approval screens, and types the dashboard's charts and the topology graph against d3's own types.
PatternFly spreads the props it does not recognise straight onto the DOM element, so dataCy reached the browser as datacy and no [data-cy=...] selector matched any of these eight. The codebase's own components declare a dataCy prop and map it; PatternFly's do not.
Converts the rest of src/screens, layer by layer: the small screens, the resource lists, hosts, instances and organizations, credentials, projects and instance groups, jobs and users, templates, settings, inventories, and finally the application entry and its setup. No JavaScript is left outside the tests.
Converts all 553 test files. A partial API fixture gets an explicit cast rather
than the application's own types being loosened to accept it, and a prop a test
leaves off is made optional only where the component already copes without it.
Type checking the tests turned up a set of things they were not doing:
- new Error({ response }) hands the object to the Error constructor, which
stringifies it, so the errors these tests passed carried no response at all
and the response branch never ran.
- LaunchPrompt called CredentialTypesAPI.loadAllTypes instead of stubbing it,
against an already mocked module, so the credential types were never there.
- NodeAddModal, NodeEditModal and EC2SubForm read everything off their context
or their form, so the props three tests passed them went nowhere.
- Two render helpers returned { history, ...utils }, overwriting the history
the test had just created with the one the render answered with.
- ScheduleForm's edit test spread the schedule over the inventory override it
meant to apply, and a visualizer action still carried the i18n object the
reducer stopped taking when it moved to a start label.
- ConstructedInventoryHint replaced the whole config module, which left the
render helper without the provider it reads at import time.
Two prop types were wrong rather than merely strict: CredentialLookup declared
credentialTypeId as a string where every caller passes the id off the API, and
PreviewStep declared formErrors as the error map an earlier API passed instead
of the flag the step is given.
Turns off allowJs, now that src and testUtils hold no JavaScript, and adds a ui-type-check job beside ui-lint so the types stay honest the way the lint rules do. It is a required check rather than an advisory one, since the tree checks clean. Replaces CONTRIBUTING's PropTypes section, which described a library the UI no longer carries, with how props are typed now, what Untyped marks, and where the API types come from. The file names the docs cite move with the conversion: index.tsx, App.tsx, rtlContexts.tsx, Base.ts, and the compiled catalogue's real name, messages.mjs.
The redirect away from /settings is a <Navigate>, which react-router applies in an effect. The test waited for the loading state to clear and then read the location, so under load the assertion could run before the navigation had been applied. Present since the initial import; it fails perhaps one run in five on a busy machine.
Narrows the first wave of Untyped markers in the sources, from 3,259 to 3,038. The props that are plainly one thing now say so: rowIndex is a number, readOnly a boolean, detailUrl a string, and name, title, tooltip and helpText are a string or a node depending on what the component does with them. The props holding an API object take the generated type for it: Host, Inventory, Project, Organization, Credential, User, Team, InstanceGroup and ExecutionEnvironment. summary_fields gains the members the UI reads and the hand-written types did not name: the roles an object offers, the job that last ran, the groups a host is in, who an activity stream entry is about. Three more fields are named because the serializers build them in method fields the schema cannot see: an organization's galaxy_credentials, a user's auth, and the webhook_key the project form fetches from its own endpoint. What the narrower types found: - The rows of nine lists were passing PatternFly no row index, so every one of their checkboxes was labelled "Select all rows" and the selection cell was not announced as a cell at all. Two tests asserted that as the expected shape. - AnsibleSelect wrote its name onto the change event whether it had one or not, so three selects that do not use it set the DOM attribute to the string "undefined". - Two role lists ran parseInt over an id the API already sends as a number. - Reading a summary field the API had not sent threw rather than rendering nothing, in twenty-four places.
Narrows the second wave of Untyped markers in the sources, from 3,038 to 2,903. useState<Untyped> said nothing and useState(null) inferred null, so either way the setter took nothing the component could give it. The error slots now hold unknown, which is what a caught error is until something asserts a shape on it, and the rest hold what they are set to: a boolean, a path string, a count, a map of which wizard steps have been visited, the source control types the API offers. The handlers a component calls with nothing now say so, rather than declaring a rest parameter that allowed any arity. onBlur keeps its event, because formik hands its own blur handler straight through. What the narrower types found: - The launch prompt's labels field set the caught error as the field's error message and rendered it, so a failed label lookup threw "Objects are not valid as a React child" instead of showing anything. - Four forms held their content error in a state initialised to false or null and set it to the error object, which is what ContentError is then given.
… user Narrows the third wave of Untyped markers in the sources, from 2,903 to 2,863. getSearchableKeys answers with the fields a list can be filtered on and the type each one holds, which is now a named SearchableKey rather than four different guesses at its shape along the way: unknown[], string[], string[][] and Untyped all described the same array. The related keys beside it are plain field names, and say so. The current user off /api/v2/me is a named CurrentUser, exported from the config context that fetches it, and the six props that pass it around take it. setBreadcrumb is a function rather than anything at all, and the two screens that build a trail three levels deep declare the third argument.
The test files were outside eslint before this branch, and the conversion kept them there rather than making that decision. This brings them in under the same rules, less the handful that describe an application rather than a test of one: an assertion names the string the user sees, a test file has no default export to be its subject, the console is what several of them silence, and a stub component reads whatever the real one is given. Two hundred and eighty errors came out of it. Most were import order and duplicate imports, which are mechanical. The rest were real: nineteen imports and bindings nothing used, a component stub that called a hook from a function named "default", a catalogue imported by the file name the runner rewrites rather than the one everything else uses, and an entry imported as ./index.js from a directory that has no such file.
Choosing a node type set ten formik fields one after another, so there were renders in between where the type had changed and the rest of the step had not. The list for the new type mounts in that window, and a resource selected there is undone by the reset still arriving behind it: the radio ticks, then clears. A person cannot click inside those microseconds, which is why this only showed up in the test suite, where it failed about one run in five. The fields are set together now, so there is no window to click in. The test that caught it waited for a row selector the previous type's list also satisfied, so it could click a row that was about to be replaced. It waits for a row of the type it has just chosen instead.
Narrows the fourth wave of Untyped markers in the sources, from 2,863 to 2,811. A row index is a number, a list's query parameters are the QSParams the query string helpers build, and a workflow node is the WorkflowNode the reducer holds. The jobs are three different shapes and were all one marker: a list row is a UnifiedJob, the job's own screen reads a Job, and what the sparkline draws are the summary field's RecentJob entries, which carry four fields rather than a serializer. Saying which is which is what the sparkline, the project row and the traceback loader now do.
…inks Narrows the fifth wave of Untyped markers in the sources, from 2,811 to 2,787. A launch config and a survey config are the LaunchConfig and SurveyConfig the prompt steps already declare, a list's query config is the QSConfig its helpers build, and an organization or inventory id is a number or a string. The schedule screens say a resource that cannot be prompted has no launch or survey config, and now the screens below them say the same rather than declaring both required and being handed undefined. The wizard asserts them at the one boundary where having them is what opening it means.
Narrows the sixth wave of Untyped markers in the sources, from 2,787 to 2,760. Adds the aliases the conversion had not needed yet, from the same generated schema as the rest: Group, CredentialType, InventorySource, Instance, NotificationTemplate, WorkflowApproval, OAuth2Application and OAuth2Token. The props that hold one of them take it. Three more fields are named because the serializers build them where the schema cannot see: a credential type's inputs and injectors are JSONFields, a workflow approval carries the timeout of the template it came from, and its user_has_voted is a boolean that the schema describes as a string. What the narrower types found: - An instance's capacity adjustment comes off the API as a decimal string, and went to PatternFly's slider and to computeForks as one. The slider took it as a value it could not step from: a keyboard right arrow jumped to the maximum rather than moving one step, which its test had written down as a jsdom quirk. - The elapsed seconds on a workflow approval are a string too, and were being handed to a function that formats a number. - An external credential type's test dialog read inputs.metadata and inputs.required without checking that this type declares any.
Narrows the seventh wave of Untyped markers in the sources, from 2,760 to 2,758. Both forms add as well as edit, so they hold part of a template or none of it, which Partial says. The workflow template gains the webhook_key its own endpoint answers with, the same way the project did. Two things in their seeds were wrong rather than merely untyped. The workflow form seeded a project, which a workflow job template does not have: the field was copied from the job template form beside it and went nowhere. The job form seeded summary_fields.inventory and summary_fields.project as null, where the API omits a related object it has none of, and every read of them is a truthiness check either way.
The chart builds the tooltip with a `t`, the constructor stored `opts.i18n`, and draw() reads `this.t`. Nothing ever set it, so constructing the tooltip threw on its first label and the usage chart drew nothing. Present since the initial import. Types the two d3 charts while the area was open. The dashboard's points are a named ChartPoint and the subscription usage chart's a UsagePoint, so the line generators, the scales and the mouse handlers all say which they hold, and the data the screens assemble for them says what a day and a month carry. The dashboard chart's tooltip also ran the same four assignments twice, one block after the other.
The wizard resets the form once the launch config and survey have loaded, and carries the chosen resource across the reset. It read that resource out of the values its effect last closed over, and the effect's dependency list leaves the form out on purpose, so the values it saw could predate the click that chose the resource: the row ticked and then untick itself when the config landed. In a browser the fetch outlives the render that follows the click, so the values were usually fresh by the time the reset ran. The test suite answers from a mock, which is what made it visible. Also names the metrics chart's series and readings, and the breadcrumb trail the screens hand up, so what a screen passes its parent is a shape rather than anything at all.
Narrows the eighth wave of Untyped markers in the sources, from 2,567 to 2,543. Every handler the conversion wrote as (...args: Untyped[]) => void now says what it is called with, which is what turns a wrong argument order into a compile error rather than a rendering oddity. Two names meant two different things and were declared the same way. onRemove removes a search term from a list's filters, taking the key and the value, and removes the selected item from a chip group, taking the item. onConfirm on the link modals confirms with the link type and, for a conditional link, the condition beside it.
Narrows the ninth wave of Untyped markers in the sources, from 2,543 to 2,489. The output tree is the largest piece of untyped state left in the UI: a job's events indexed by counter, the collapsible tree over them, the events whose parent has not arrived yet, and the row each one draws on. All of that is named now, along with the actions that change it and the callbacks the screen gives the tree so it can fill its own gaps. Two things fell out of naming the actions. The toggle a row dispatches carried a collapsed flag beside the uuid, which the reducer never read: it toggles the node it is given. And a row's counter lookup answers with its best guess for a row that has not loaded, which the loader was using without saying so.
The inventory form names its values, so the add and edit screens that pull the organization and the labels out of them are checked against what the form holds, and the form no longer takes a credentialTypeId nothing reads. The inventory source hook takes a source and a websocket message rather than two markers.
Both name their values, and the constructed inventory's options are the POST field map the endpoint answers with rather than a loose record: the model that reads them says so now too. The two group add screens set the inventory onto the values object they were handed before posting it; they pass it alongside instead.
The five node type lists take the template a node runs, which is a named type; it had to widen its job_type, because a node can run a system job template and those name a cleanup task where a job template names a run or a check. The hook tests keep their result in a slot that names the hook's return type, so useRequest's and useDeleteItems' results are told apart rather than both being the marker.
A screen's test puts a stub in the form's place and keeps what it was handed, so the assertions can say what the screen passed; the slot names those props rather than holding the marker. Two list row tests were rendering rows without the rowIndex and detailUrl their components require, and the schedule row test passed strings where the component takes the two booleans its list passes.
The form names its values, which is what withFormik carries into the add and edit screens: both were building the api payload by assigning onto the object they had destructured the rest of the values into, and pass a payload of their own now. The component's own handleSubmit is formik's, taking the event, not the caller's prop of the same name that withFormik shadows; the props say so.
The options block a settings screen is given names its unit as null where the api sends one, which is most of them, so the field and the detail that shows it say so. The Azure AD tenant edit test built its own settings block and added the five tenant fields at the top level, where the screen reads them off PUT: the fixture already carries them there, so the override was never reaching the screen.
The user form names both what it holds and what it posts: the two are not the same, because the confirm password and the preferred theme never leave the form, and the edit screen was deleting the organization off formik's own values object rather than off a copy. The host form's values were the marker, so its edit screen assigned an id over the lookup's whole object in place; it builds its payload now. The project row names the four handlers its list passes, and the row test was rendering it without any of them.
can_approve_or_deny is a SerializerMethodField the schema describes as a string; it joins user_has_voted as an override, because everything that reads it reads a boolean. The workflow output toolbar reads the template off the job's summary as an object in one place and as a list in another, which the api does send both ways: it says so rather than reaching for both through a marker. The node modal's waits carry this file's own timeout. The wizard refetches on every step, and under the full run's parallelism the suite default was not always enough: that file failed about one run in three.
The inventory source read returns the source rather than a bare entity, so the screen that reads it gets a type; the model said ApiEntity where it answers with one row of its own list. The host add and the inventory host edit each assigned an id over the lookup's whole object on formik's own values before posting, the same shape the host edit had; both build a payload instead. The group and inventory rows read user_capabilities unguarded where the api leaves it out, which is every place a list is rendered before its options have come back.
The inventory source prompt detail renders Regions, Instance Filters and Only Group By, which were columns on the inventory source until the inventory plugins migration dropped all three: 0119_inventory_plugins removes them from both inventorysource and inventoryupdate, and nothing in the platform has sent them since. The marker was what let the reads compile, and the test's fixture still carries them from before that migration. The remaining component props and test helpers are typed alongside.
The wizard, table and icon stubs this suite stands in for PatternFly with pass their props straight through to a DOM element, which is one named type, and the wizard's children are its steps, which it reads an id and a name off.
Three more edit screens built their api payload by assigning onto the object they had destructured the rest of the form values into; they pass a payload alongside now. The user token list builds a name onto each row for the delete confirmation, which the api does not send, the same way the application's does. The usage chart tooltip's eleven appended elements each name their own, for the same reason the dashboard's did.
SelectedList is generic in the row it holds, so a lookup that names its own type gets it back in the chip renderer rather than the widest thing a list can hold. The activity stream row set the table row's id to the entry's id, a number where the attribute is a string, the same as the related group row did. The notification list's toast takes the notification the test run produced, not the template it was sent from.
Narrows the migration markers by another 54, across the last of the components that the lookups, the template lists and the workflow output graph share. Three findings behind the changes: DraggableSelectedList took an onRowDrag prop, read it nowhere and rendered no drag handles. The drag-and-drop went out with the PatternFly v4 to v5 upgrade (ctrliq#506), which removed onDragFinish and the list's "Draggable list to reorder and remove selected items." label, and it was never restored. The prop and the four sortSelectedItems callbacks that fed it are dead wiring, so they go; reordering a selected list stays lost, and the credential lookup's "Select more than one to enable drag" description still promises it. JobTemplateForm declared handleSubmit as the caller's value-taking submit, but withFormik injects its own over it, so what the component actually receives is formik's event handler. It was cast away twice to compile. Typed as formik's, matching WorkflowJobTemplateForm, and the casts go with it. resourceValues was reaching the form through the props index signature; it is the resource an add screen was reached from, so it gets a name and a type of its own. WorkflowOutputNode read prior_run_succeeded and prior_run_elapsed off an index signature, because ApiWorkflowNode is built from the workflow job *template* node, which carries neither. Both are declared now, and a node's elapsed time is converted before formatting: the api sends the decimal as a string, and secondsToHHMMSS takes a number. Also: module_name's OPTIONS choices are [string | number | null, string] pairs, not [string, string], so the ad hoc wizard now takes the api's own OptionsChoice and the six inventory lists stop widening it.
Every source file under awx/ui now carries a real type: the only place the Untyped marker is still declared is types/api.ts, where the 212 remaining uses are all in test files. The ad hoc command preview never showed the credential. It reads values.credential, and the wizard's field is values.credentials, so the Credential detail has never rendered; the plural key is also excluded from the generic detail list above it, so the credential the user picked appeared nowhere on the review step. Inherited from AWX 22.5.0, where the same two lines sit in AdHocPreviewStep.js. The project add and edit submits took their values as untyped and rewrote them in place, turning each lookup's object into its id before posting. They now take the form's own values and build the request body beside them, which is what they were doing through the mutation. Their tests handed the mock form raw numeric credential ids, a shape the form cannot produce: the lookups hold the whole credential and a reset hands back an empty string. The fixtures now hold what the form holds, and the requests they assert are unchanged. Smaller ones: the copy button's onCopyStart and onCopyFinish take nothing and are called with nothing; a health check's pending flag is a boolean; a status label's tooltip is a node; the job list fetches by a list of ids; the login screen submits the two fields it collects; and the notification mixin's shared cases name the members they exercise rather than taking the model as any.
Takes the marker count from 212 to 85 across 30 test files, mostly by naming what each file's own helpers take: a matcher, a string, a set of form props, a mock. The pieces worth naming: - The mock forms that stand in for a real one now take that form's own props type, so a stub that drifts from the component it replaces stops compiling. Doing that to JobTemplateEdit's stub showed its submit values held the empty string where the form clears the execution environment to null, and carried the api's nulls for description, playbook and scm_branch where the form holds empty strings. - ScheduleForm's prompt cases built a survey question with no variable, which is not a question the api can return; the schedule fixture is cast once now rather than left implicit. - WorkflowJobTemplateEdit's submitted values were missing webhook_key, one of the form's own fields. - The credential form's fixtures are cast once where they are imported, the way the other JSON payloads in this tree already are. - The dates test carried an i18n stub nothing used, kept alive by an eslint-disable; it is gone. useRequest's delete cases passed an empty object as their queryset config, which has four required members. They build a real one now.
The Untyped marker is gone, along with the type itself. Nothing under src or testUtils is any any more, and eslint's no-explicit-any is on to keep it that way. The 85 remaining uses were all in test files, and narrowing them turned up the same kind of thing the source sweep did: fixtures that describe a payload the api does not send. - A workflow node's elapsed time and a workflow job's were both numbers in their fixtures, where the api serializes the decimal as a string. With the fixtures corrected, the Number() conversion the node detail gained in the previous commit is what keeps them formatting. - The organization form's instance groups stub called onChange with a second argument naming the field. The real lookup passes one, and the handler on the other side takes one. - Two more survey fixtures built a question with no variable. - The host add and edit screens parked their stubbed form's submit values on the global as an untyped slot; it is declared now, and the edit screen's failing-submit case stopped overwriting it with a host payload where form values belong. Each mocked endpoint's response is named after the endpoint itself (ResponseOf<typeof SomeAPI.method>) rather than ApiResponse<any>, which is what the rest of the suite already did. The two places any remains are named and explained: Base's mixin constructor, which TypeScript requires to take any[], and nothing else. The simulation worker's d3, which arrives through importScripts rather than from a package with types, now describes the force and simulation surfaces it actually calls.
Two things this branch had recorded rather than fixed. Reordering a selected list works again. The drag went out with the PatternFly v4 to v5 upgrade (ctrliq#506), which removed the DataList props it was built on, and PatternFly 6 has no replacement for them: it keeps DataListControl and DataListDragButton as the pieces, and leaves the dragging to the page. So the rows carry native HTML5 drag now, the way the survey reorder modal in this tree already does under PatternFly 6, and the four lookups that were handing their sortSelectedItems to a prop nobody read are wired back up. The credential lookup's "Select more than one to enable drag" means something again. Two things the old implementation got wrong are not carried over. Its drag handle gave both aria-label and aria-labelledby, and labelledby wins, so every handle was announced as the row's text rather than as what it does; each one is now "Reorder <name>". And it passed the keyboard instructions as aria-describedby text, where that attribute takes an id: the instructions are rendered once per list and every handle points at them. The keyboard reorder those instructions promise is implemented rather than promised: space or enter picks a row up, the arrow keys move it, escape puts it back. The inventory source form carried subforms for insights and rhv, and the project form one for insights. The platform comments all three out of SOURCE_CHOICES and SCM_TYPE_CHOICES, so the api refuses every one: the subforms could not be reached and the list filters offering those types could only return nothing. Both subforms and the five filters go, along with two doc urls and three test fixtures that still listed them. The insights credential kind is untouched: that one the platform still has. So is the job detail's scm type label, which names what a row holds rather than offering a choice.
Four guards added while typing sat inside a Plural's value, and lingui
names a placeholder after the expression it is given: a bare identifier
becomes {name}, anything else becomes {0}. So `sessionCountdown ?? 0`
turned {sessionCountdown} into {0}, `failedSources` turned {0} into
{failedSources}, and `forks ?? 0` did it twice. A changed message id
matches nothing in the catalogues, so each one silently dropped that
string back to English in all eight translated locales.
None of the four guards was reachable. sessionCountdown is declared a
number rather than an optional one, and both forks screens return their
loading state long before the Plural renders, so the state is seeded
with zero instead of guarded at the point of use. The inventory list
passes the expression the id was built from rather than the local
beside it, with a comment saying why, because that one has to stay an
expression to keep naming its placeholder {0}.
The catalogues are re-extracted and recompiled with that put right.
Every file in this tree was renamed by the conversion, so every source
reference in the nine catalogues pointed at a path that no longer
exists; the new strings the restored drag handle needs are picked up in
the same pass. Nothing translated is lost: the thirty-seven ids that go
are strings no longer in the source, the expand button one of them,
whose absence a test now asserts by name.
Twelve strings arrive that predate this branch. They are in main's
source and were never in main's catalogue, so running the extractor at
all brings them in.
A message id is built from the string and from the names of whatever is
interpolated into it, so an edit as small as guarding a value changes
one: `<Plural value={forks} />` names its placeholder {forks} and
`<Plural value={forks ?? 0} />` names it {0}. The renamed id matches
nothing in the catalogues and the string falls back to English in every
translated locale, while types, lint, tests and the build all stay
green. Four of those reached this branch and were found by hand.
`npm run check-strings` extracts against a throwaway catalogue and
compares the ids with the ones checked in, so the working tree is left
alone whether it passes or fails. ui-lint runs it, which makes it part
of a check that already has to pass.
Ids are compared rather than files: the catalogues carry source
references with line numbers, so comparing them whole would fail on any
change that shifts a line. They are counted rather than collected into
a set, because one id can hold two entries, lingui keeping a message
given an explicit id apart from one whose id it generated from the same
text; a set would hide one of such a pair going missing. Verified by
putting the sessionCountdown guard back and watching it fail, naming
both sides of the rename.
CONTRIBUTING gains the rule this enforces, which is to guard a value
where it is made rather than where it is read, and loses a typo that
had been pointing at `npm run extra-strings`.
The check already failed closed on a source file the extractor cannot parse, which is the behaviour that matters, but node reports a failed child process by dumping its stdio buffers, so what reached the log was several thousand numbered bytes rather than a reason. It now prints what the extractor said, which names the file and the parse error in it.
package.json asked for node >=22.22.0 while jsdom, which the test run depends on, asks for ^22.22.2 || ^24.15.0 || >=26.0.0. The ranges disagree over 22.22.0 and 22.22.1, the whole of 23, and the whole of 25, and awx/ui/.npmrc sets engine-strict, so npm install is a hard error on every one of them. The project was promising support for node versions on which it cannot be installed at all. The range is jsdom's now, that being the binding constraint. A node this tree cannot be built on is refused against what the project itself asks for rather than against a transitive dependency, which is the difference between being told to use another node and being told something about jsdom. Node 25 is genuinely out. jsdom 29 took anything from 24 up, and 30 narrowed to the long term support lines, so the only way back to 25 would be to give up a major version of jsdom. .nvmrc names 22, the line the devel image builds with, so nvm picks a supported one by itself.
IRC over SSL is broken today. The backend hands `ssl.wrap_socket` to the connection factory, and that function went away in Python 3.12 while the images run 3.14, so any notification to an SSL server raises. A notification only needs to register, join and send, which is a hundred lines of `socket` and `ssl`. The backend now does that itself: it answers PING during registration, waits for the welcome reply before sending, joins a channel before messaging it, and gives the whole exchange the same sixty second budget the reactor loop had. That takes the `irc` package out of the tree and fourteen packages with it, since nothing else depends on them: typer, rich, pygments, the five jaraco modules, markdown-it-py, mdurl, shellingham, tempora, typer-slim and pytz. Their license files go too, and jaraco-functools stays because requirements.in asks for it directly. The backend had no unit test, so this adds one: a scripted socket covers joining a channel, messaging a nick, answering PING, and both the raising and fail_silently paths of a refused connection. requirements.txt was pruned by provenance rather than recompiled, because requirements/updater.sh pins python3.14 and this machine has 3.12. Running it should be a no-op, and is worth doing before merge.
pygerduty exists here to make a single call, trigger_incident, which is a POST to the Events API v1 endpoint. The backend now makes that POST itself with requests, which the other HTTP notification backends already use. The payload is what pygerduty sent: service_key, event_type trigger, description, details and client. The request carries the standard client headers and AWX_NOTIFICATION_REQUEST_TIMEOUT, so unlike the old call it cannot hang a dispatcher worker. subdomain and token stay as init parameters. They belong to the REST API rather than to triggering an incident, so they were never part of this call, and removing them would change the credential shape of every PagerDuty notification template that exists. The backend had no unit test; this adds one covering the payload, the endpoint, the timeout, and both the raising and fail_silently paths.
The twilio SDK is here for a single call, messages.create, which is a form-encoded POST to the Messages endpoint with the account SID and token as basic auth. The backend now makes that POST with requests. Same fields, To, From and Body, and the same handling of a partial failure: the first exception is kept and raised after every recipient has been tried, rather than at the point it happened. Two things the SDK was hiding. There is no client to construct, so the branch that caught a construction failure and then used the unbound connection anyway is gone. And the request carries AWX_NOTIFICATION_REQUEST_TIMEOUT, so a stalled endpoint can no longer hold a dispatcher worker. twilio leaves the tree and takes aiohttp-retry with it. aiohttp stays, since requirements.in and kubernetes both ask for it, and its via list is updated to say so. test_twilio.py mocked the SDK client and now mocks the POST, keeping the three cases it already covered.
The asn1 package is in the tree for four lines in one function: encoding the instance hostname as a UTF8String so it can be carried as the value of the receptor OID in the certificate's subject alternative names. That encoding is a tag byte, a length, and the payload. cryptography takes the OtherName value as bytes and has no public encoder of its own, so the helper is written here rather than borrowed, with the long form handled for completeness even though a hostname never reaches 128 bytes. Checked against the package it replaces before removing it: the two agree byte for byte on the empty string, a single character, a hostname, 127 and 128 byte payloads either side of the short form boundary, 300 bytes, 65535 bytes, and a multibyte string where the length counts bytes rather than characters.
Contributor
Author
|
Duplicate of #921, which came first and does it better: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sits on top of #925, and through it #924, #923, #919 and #920. Fourth of the series from the "same feature, smaller footprint" list on the roadmap thread.
Problem
asn1is a dependency for four lines ingenerate_receptor_tls: encoding the instance hostname as a UTF8String so it can be the value of the receptor OID in the certificate's SANs. Nothing else in the tree writes ASN.1.Change
A
der_utf8_stringhelper in the same module. The encoding is a tag byte, a length and the payload, with the long form handled even though a hostname never reaches 128 bytes.Worth correcting something I wrote on the roadmap thread: I said
cryptographyalready does this in a few lines. It does not. It takes theOtherNamevalue as encoded bytes and exposes no general encoder, so the helper is written here rather than borrowed from it.Checks
Before removing the package, the helper was compared against it byte for byte on eight inputs: the empty string, one character, a hostname, 127 bytes and 128 bytes either side of the short form boundary, 300 bytes, 65535 bytes, and a multibyte string where the length must count bytes rather than characters. They agree on all eight.
awx/main/tests/unit/api/test_instance_install_bundle.pykeeps six of those as a parametrised test plus the multibyte case, asserting literal expected bytes so the test does not depend on the package being removed.ruff checkandruff format --checkclean.requirements.txtpruned by provenance rather than recompiled, since the updater pins python3.14 and this machine has 3.12;asn1had no dependencies of its own.Not tested here
Generating a real install bundle and having receptor accept the certificate. The encoder is verified against what the package produced, so the bytes in the SAN are unchanged, but an end to end bundle is worth generating before release.