Add unsafe-dependency bypass flags to om apply-changes - #822
Open
skaarthik wants to merge 5 commits into
Open
Conversation
Context: Tiles can declare optional dependencies on other tiles. Ops Manager blocks apply-changes when an optional dependency has unsafe property changes on update, or when deleting a product whose optional dependent isn't marked safe-to-delete. Platform engineers had no way to proceed past these specific checks from the CLI even when they understood and accepted the risk — the only path was disabling verifiers more broadly via --ignore-warnings, which suppresses everything overridable rather than just this one class of check. Implementation notes: - Added --allow-unsafe-dependency-update and --allow-unsafe-dependency-deletion flags to `apply-changes`, wired through ApplyChanges.Options -> applyChangesService -> Api.CreateInstallation. - Both default to false and are marshaled with `omitempty`, so they are only present in the POST /api/v0/installations body when set to true — the request is byte-for-byte unchanged for every caller that doesn't pass them. - This is a CLI-only change. The Ops Manager backend support for these two params (permitted on the installations endpoint, wired through ValidationOverrideOptions into ProductDependencyVerifier) already exists as of Ops Manager releases/11.0 (introduced 2026-06-18); this branch only makes `om` use what's already there. Against an Ops Manager version predating that release, the fields are silently stripped by Rails strong-parameter filtering rather than causing a request error — the flags simply have no effect on those older targets. - On a blocked apply-changes, Ops Manager's 422 response includes an `available_overrides` array naming which bypass(es) would unblock the specific failure. CreateInstallation now decodes that and appends a hint mapping known values to their CLI flag (falling back to the raw value for any override `om` doesn't recognize yet, per the API's forward-compatibility contract). - Regenerated the counterfeiter fake for applyChangesService to match the new signature; regenerated docs/apply-changes/README.md via the repo's docsgenerator tooling rather than hand-editing it. Usage notes: - `om apply-changes --allow-unsafe-dependency-update` - `om apply-changes --allow-unsafe-dependency-deletion` - Both can be combined with each other and with existing flags (e.g. --product-name). Both are no-ops unless explicitly passed. - If apply-changes is blocked and the failure has a known override, the resulting error now suggests the specific flag to retry with, instead of only pointing at `om pre-deploy-check`. - Requires an Ops Manager release that includes this backend feature (11.0+); on older targets, passing these flags will not bypass anything and the original blocking behavior still applies.
commands/apply_changes_test.go 1. Extended the baseline "applies changes to the Ops Manager" test to assert allowUnsafeDependencyUpdate and allowUnsafeDependencyDeletion both default to false when neither flag is passed. 2. New: "passed both the allow-unsafe-dependency-update and allow-unsafe-dependency-deletion flags" — asserts both reach CreateInstallation as true and both log lines fire. api/installations_service_test.go 1. New: "allowing both unsafe dependency update and deletion" → "includes both allow_unsafe_dependency_update and allow_unsafe_dependency_deletion in the request" — asserts both JSON keys are sent together and both log lines fire. 2. New: "suggests --ignore-warnings when available_overrides includes ignore_warnings" — covers the previously-untested ignore_warnings entry in overrideFlagHints. 3. Extended: "falls back to the raw parameter name for an unrecognized override value" — added a stdout log assertion (previously only checked the returned error). 4. Extended: "does not append an override hint when available_overrides is empty" — added a negative stdout assertion (ToNot(gbytes.Say(...))) confirming no hint is logged either. 5. New: "does not append an override hint when the response body cannot be decoded as JSON" — a dedicated malformed-body (not-valid-json) case verifying availableOverridesHint degrades gracefully.
sanjimoh
reviewed
Sep 7, 2026
Context: Code review flagged that the CLI printed "checks will be bypassed" purely based on the --allow-unsafe-dependency-* flags being parsed, with no check that the targeted Ops Manager actually understands them. Against an unsupported target, the message was simply false: the field would be silently dropped server-side, the original safety check would still fire, and the user would see the exact error they were trying to bypass with no indication why the flag they'd just been told "will be bypassed" did nothing. Implementation notes: - commands/apply_changes.go: added a version gate that fails fast with a clear error when either new flag is used against an Ops Manager older than 11.0 — the first release confirmed to accept allow_unsafe_dependency_update / allow_unsafe_dependency_deletion on POST /api/v0/installations. - Mirrors the existing --product-name version check exactly: same Info() + VersionAtLeast() call, same early placement in Execute() before RunningInstallation()/RecreateVMs side effects run, so a rejection here never leaves Ops Manager in a partially-mutated state. - Both flags are covered by a single combined `if (AllowUnsafeDependencyUpdate || AllowUnsafeDependencyDeletion)` with one early return, so passing both together against an unsupported Ops Manager produces one message naming both flags, not two concatenated errors. - The existing "checks will be bypassed" log statements are unchanged in wording — they're now only ever reached once version support is confirmed, so they stop asserting something that isn't true rather than needing to be reworded. Usage notes: - --allow-unsafe-dependency-update / --allow-unsafe-dependency-deletion now require Ops Manager 11.0 or later. Using either against an older target fails immediately with: "--allow-unsafe-dependency-update and --allow-unsafe-dependency-deletion are only available with Ops Manager 11.0 or later: you are running <version>." — no installation is attempted, and no misleading "will be bypassed" message is printed first. Other notes: - Test coverage added for the gate: each flag individually against Ops Manager 10.2, and both flags together against Ops Manager 10.2 — each asserting CreateInstallation is never called and the "will be bypassed" message never appears. The combined case also asserts the error text contains exactly one "only available with Ops Manager" / "you are running" occurrence, directly verifying the message isn't duplicated across both flags. - Known limitation: this gate lives in commands/apply_changes.go's Execute(), not inside api/installations_service.go's CreateInstallation itself. It protects the om apply-changes CLI path — currently the only caller of CreateInstallation in this codebase — but CreateInstallation has no version check of its own. Any future caller (another command, or a consumer importing the api package directly) would bypass this guard entirely and could send these flags to any Ops Manager version unguarded. If that guarantee needs to hold regardless of caller, the check should move into (or be duplicated in) CreateInstallation. TNZ-144620 ai-assisted=yes
sanjimoh
reviewed
Sep 7, 2026
…more unit tests commands/apply_changes.go: split the version-gate error into two distinct messages — "Could not determine Ops Manager version..." when the version string can't be parsed, vs. "...are only available with Ops Manager 11.0 or later: you are running <version>" when it's parseable but too old — removing the previous message's trailing Error: %!w(<nil>) artifact from wrapping a nil error. commands/apply_changes_test.go — baseline test: added assertion that "checks will be bypassed" never appears in stderr when neither new flag is passed. commands/apply_changes_test.go — tightened the existing "too old" test from ContainSubstring to an exact MatchError, locking in the fixed message has no trailing artifact. commands/apply_changes_test.go: added negative tests for the "version cannot be determined" branch — individually for --allow-unsafe-dependency-update and --allow-unsafe-dependency-deletion, and combined (asserting exactly one non-redundant message when both flags are passed together). commands/apply_changes_test.go: added negative tests for ac.service.Info() itself failing (e.g. connection failure) — individually for each flag — asserting the correct error, zero CreateInstallation calls, and no misleading "will be bypassed" log line. api/installations_service_test.go: added assertions to the existing "omits ... when false" tests that the corresponding "allow_unsafe_dependency_update=true" / "...deletion=true" log line is absent when the flag is false. TNZ-144620 ai-assisted=yes
sanjimoh
approved these changes
Sep 8, 2026
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.
Context:
Tiles can declare optional dependencies on other tiles. Ops Manager blocks apply-changes when an optional dependency has unsafe property changes on update, or when deleting a product whose optional dependent isn't marked safe-to-delete. Platform engineers had no way to proceed past these specific checks from the CLI even when they understood and accepted the risk — the only path was disabling verifiers more broadly via --ignore-warnings, which suppresses everything overridable rather than just this one class of check.
Implementation notes:
apply-changes, wired through ApplyChanges.Options -> applyChangesService -> Api.CreateInstallation.omitempty, so they are only present in the POST /api/v0/installations body when set to true — the request is byte-for-byte unchanged for every caller that doesn't pass them.omuse what's already there. Against an Ops Manager version predating that release, the fields are silently stripped by Rails strong-parameter filtering rather than causing a request error — the flags simply have no effect on those older targets.available_overridesarray naming which bypass(es) would unblock the specific failure. CreateInstallation now decodes that and appends a hint mapping known values to their CLI flag (falling back to the raw value for any overrideomdoesn't recognize yet, per the API's forward-compatibility contract).Usage notes:
om apply-changes --allow-unsafe-dependency-updateom apply-changes --allow-unsafe-dependency-deletionom pre-deploy-check.