fix(aws): give the permission check room for throttled retries and say it can be slow
Secret Scan / scan (push) Successful in 5s
Build App (Preview) / compute-version (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 2s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / build-macos (pull_request) Successful in 5m56s
Build App (Preview) / build-linux (pull_request) Successful in 8m42s
Build App (Preview) / build-windows (pull_request) Successful in 10m38s
Build App (Preview) / prune-previews (pull_request) Successful in 3s
Build App (Preview) / test (pull_request) Successful in 22m44s
Build Container / build-container (pull_request) Successful in 25m25s
Secret Scan / scan (push) Successful in 5s
Build App (Preview) / compute-version (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 2s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / build-macos (pull_request) Successful in 5m56s
Build App (Preview) / build-linux (pull_request) Successful in 8m42s
Build App (Preview) / build-windows (pull_request) Successful in 10m38s
Build App (Preview) / prune-previews (pull_request) Successful in 3s
Build App (Preview) / test (pull_request) Successful in 22m44s
Build Container / build-container (pull_request) Successful in 25m25s
The connection-wide 30 s operation limit counts retry backoff, so a throttled SimulatePrincipalPolicy got about six of its ten adaptive attempts. The IAM client used by the check now has a 120 s operation limit, and Settings tells the user a check can take a few minutes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
3c1c276769
commit
276f5378a4
4 files changed
+54
-2
No files matched your search
@@ -135,8 +135,13 @@ impl SdkPermissionApi {
|
||||
// hundred simulations) failed a third of its rows with "Rate
|
||||
// exceeded" under standard retries (verified live). Adaptive retry
|
||||
// backs off client-side once throttled, with room to retry.
|
||||
//
|
||||
// The connection's 30 s operation limit counts the retries' backoff
|
||||
// too, which would cut those ten attempts to about six; the check
|
||||
// gets its own, longer limit, and the UI says it can take a while.
|
||||
let iam = aws_sdk_iam::config::Builder::from(cfg)
|
||||
.retry_config(aws_config::retry::RetryConfig::adaptive().with_max_attempts(10))
|
||||
.timeout_config(iam_check_timeouts())
|
||||
.build();
|
||||
SdkPermissionApi {
|
||||
iam: aws_sdk_iam::Client::from_conf(iam),
|
||||
@@ -147,6 +152,17 @@ impl SdkPermissionApi {
|
||||
}
|
||||
}
|
||||
|
||||
/// The whole of one simulation, retries and backoff included. Ten adaptive
|
||||
/// attempts with backoff capped at 20 s fit in two minutes.
|
||||
pub const IAM_CHECK_OPERATION_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(120);
|
||||
|
||||
fn iam_check_timeouts() -> aws_config::timeout::TimeoutConfig {
|
||||
aws_config::timeout::TimeoutConfig::builder()
|
||||
.operation_timeout(IAM_CHECK_OPERATION_TIMEOUT)
|
||||
.operation_attempt_timeout(super::connection::ATTEMPT_TIMEOUT)
|
||||
.build()
|
||||
}
|
||||
|
||||
impl PermissionApi for SdkPermissionApi {
|
||||
async fn resolve_principal(&self, principal: PrincipalArn) -> Result<String, AwsError> {
|
||||
identity::resolve_principal(&self.iam, principal).await
|
||||
|
||||
@@ -628,3 +628,13 @@ fn no_probe_is_cloudformation() {
|
||||
}
|
||||
assert_eq!(Probe::for_action("cloudformation:DescribeStacks"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_iam_check_gets_room_for_its_throttled_retries() {
|
||||
let t = iam_check_timeouts();
|
||||
assert_eq!(t.operation_timeout(), Some(IAM_CHECK_OPERATION_TIMEOUT));
|
||||
assert!(IAM_CHECK_OPERATION_TIMEOUT > crate::aws::connection::OPERATION_TIMEOUT);
|
||||
// Ten adaptive attempts, backoff capped at 20 s, fit inside it.
|
||||
assert!(IAM_CHECK_OPERATION_TIMEOUT >= std::time::Duration::from_secs(9 * 20 / 2 + 20));
|
||||
assert_eq!(t.operation_attempt_timeout(), Some(crate::aws::connection::ATTEMPT_TIMEOUT));
|
||||
}
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { fireEvent, render, screen, waitFor, within } from "@testing-library/react";
|
||||
import HostsSettings from "./HostsSettings";
|
||||
import HostsSettings, { CHECK_IN_PROGRESS } from "./HostsSettings";
|
||||
import { useAppState } from "../../store/appState";
|
||||
import type { AppSettings, HostConnection, PermissionReport } from "../../lib/types";
|
||||
|
||||
@@ -140,6 +140,21 @@ describe("HostsSettings", () => {
|
||||
expect(screen.getByLabelText("Secret access key")).toHaveValue("");
|
||||
});
|
||||
|
||||
it("tells the user a permission check can take a few minutes while it runs", async () => {
|
||||
let finish: (r: PermissionReport) => void = () => {};
|
||||
invoke.mockImplementation(async (cmd: string) => {
|
||||
if (cmd === "list_aws_profiles") return [];
|
||||
if (cmd === "check_aws_permissions") return new Promise<PermissionReport>((res) => (finish = res));
|
||||
throw new Error(`unexpected ${cmd}`);
|
||||
});
|
||||
render(<HostsSettings />);
|
||||
fireEvent.click(screen.getByRole("button", { name: "Check Prod" }));
|
||||
expect(await screen.findByRole("status")).toHaveTextContent(CHECK_IN_PROGRESS);
|
||||
finish(report({}));
|
||||
expect(await screen.findByText("Insufficient")).toBeInTheDocument();
|
||||
expect(screen.queryByText(CHECK_IN_PROGRESS)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("check shows Insufficient with the denied actions and every policy file to copy", async () => {
|
||||
const adminFiles = [
|
||||
{ name: "environment-admin.json", json: '{"Statement":"admin"}' },
|
||||
|
||||
@@ -26,6 +26,10 @@ import SegmentedControl from "../ui/SegmentedControl";
|
||||
import Field, { inputClass, selectClass } from "../ui/Field";
|
||||
|
||||
|
||||
/** AWS throttles the permission simulator; a check retries patiently instead of failing. */
|
||||
export const CHECK_IN_PROGRESS =
|
||||
"Checking permissions… AWS limits how fast this can run, so it can take up to a few minutes.";
|
||||
|
||||
const OWN_PRINCIPAL_ACTIONS = ["iam:GetRole", "iam:SimulatePrincipalPolicy"];
|
||||
|
||||
const SUMMARY_LABEL: Record<PermissionSummary, string> = {
|
||||
@@ -184,6 +188,7 @@ function ConnectionRow({ host }: { host: HostConnection }) {
|
||||
const [region, setRegion] = useState(host.kind.default_region);
|
||||
const [confirmRemove, setConfirmRemove] = useState(false);
|
||||
const [busy, setBusy] = useState(false);
|
||||
const [checking, setChecking] = useState(false);
|
||||
const [signingIn, setSigningIn] = useState(false);
|
||||
const [report, setReport] = useState<PermissionReport | null>(null);
|
||||
const [failure, setFailure] = useState<Failure | null>(null);
|
||||
@@ -201,6 +206,7 @@ function ConnectionRow({ host }: { host: HostConnection }) {
|
||||
|
||||
const runCheck = async () => {
|
||||
setBusy(true);
|
||||
setChecking(true);
|
||||
setFailure(null);
|
||||
try {
|
||||
setReport(await checkAwsPermissions(host.id));
|
||||
@@ -208,6 +214,7 @@ function ConnectionRow({ host }: { host: HostConnection }) {
|
||||
setReport(null);
|
||||
setFailure({ error: toAwsError(e), retry: runCheck });
|
||||
} finally {
|
||||
setChecking(false);
|
||||
setBusy(false);
|
||||
}
|
||||
};
|
||||
@@ -311,7 +318,11 @@ function ConnectionRow({ host }: { host: HostConnection }) {
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
{busy && !editing && !confirmRemove && <p role="status" className="text-xs text-[var(--text-secondary)]">Working…</p>}
|
||||
{busy && !editing && !confirmRemove && (
|
||||
<p role="status" className="text-xs text-[var(--text-secondary)]">
|
||||
{checking ? CHECK_IN_PROGRESS : "Working…"}
|
||||
</p>
|
||||
)}
|
||||
{failure && <ErrorLine failure={failure} onSignIn={signIn} signingIn={signingIn} />}
|
||||
{report && <PermissionReportView report={report} />}
|
||||
<EnvironmentsPanel connection={host} tier={report?.summary} />
|
||||
|
||||
Reference in new issue
Block a user