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

fix: identify sandboxes during authentication #862

Merged
merged 5 commits into from
Aug 2, 2023

Conversation

shetzel
Copy link
Contributor

@shetzel shetzel commented Jun 15, 2023

What does this PR do?

Enhances AuthInfo.identifyPossibleScratchOrgs() to also identify sandboxes.
Adds another public method to Org to allow querying the SandboxProcess sObject by org ID.

What issues does this PR fix or reference?

@W-11831532@

QA suggestions:
Create a sandbox from the UI. Auth to it from the CLI. Check the auth file for the org and that a sandbox specific file was created in the global dir. Attempting to delete the sandbox should work, since it can now be properly identified as a sandbox during authentication.

Copy link
Contributor

@mshanemc mshanemc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth solving the non-devhub + hasSandboxes filter problem.

src/org/authInfo.ts Outdated Show resolved Hide resolved
logger.debug(`error updating auth file for: ${orgAuthInfo.getUsername()}`, err);
}

try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would anything bad happen if you failed to update the AuthFile but then updated the SandboxConfig?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. That's most likely a bad state though so probably just abort the mission (return;) if there's an error updating the auth file. I'll make that change.

const stateAggregator = await StateAggregator.getInstance();
stateAggregator.sandboxes.set(fields.orgId, sfSandbox);
logger.debug(`writing sandbox auth file for: ${orgAuthInfo.getUsername()} with ID: ${fields.orgId}`);
await stateAggregator.sandboxes.write(fields.orgId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems safe from the ConfigFile bugs because it should only ever hit one of these.

Just be sure that the auth command is calling this identifyPossibleScratchOrgs NOT in parallel with anything else that might be touching the authFile.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked all places I could find and nothing was done in parallel so hopefully it's ok.

mshanemc
mshanemc previously approved these changes Aug 1, 2023

// TODO: return if url is not sandbox-like to avoid constantly asking about production orgs
// TODO: someday we make this easier by asking the org if it is a scratch org

const hubAuthInfos = await AuthInfo.getDevHubAuthInfos();
// Get a list of org auths that are known not to be devhubs, scratch orgs, or sandboxes.
const possibleProdOrgs = await AuthInfo.listAllAuthorizations(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it's possible that a DevHub is also a ProdOrg, you could include DevHubs in the filter.

Then instead of having to queryScratchOrg each DevHub and then catch to ask about sandbox, you could ask about everything on the first round.

sandboxProcessId: sbxProcess.Id,
sandboxInfoId: sbxProcess.SandboxInfoId,
timestamp: new Date().toISOString(),
} as SandboxFields;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the SandboxFields type wrong that timestamp isn't part of it? Can that be fixed, or at least leave a comment about why the type assertion is necessary?

@mshanemc
Copy link
Contributor

mshanemc commented Aug 2, 2023

QA Notes:

scratch

create a scratch org the normal way, then get password
logout from scratch org
re-auth to scratch org, is marked as scratch
✅ isScratch is true in the resulting file.

📓 Differences between a scratch org auth'd this way and the one created from the devHub

  1. loginUrl is the instanceUrl followed by /. org create had https://CS217.salesforce.com
  2. properties that exist after org create but not from login : created (a timestamp), createdOrgInstance, snapshot (set to null), tracksSource: true
  3. isScratch was not populated from org create

Not scratch or sandbox

logout, re-auth to an existing prod org, inspect auth file.
✅ isDevHub
✅ not scratch or sbox

sbox

TBD create sbox the normal way
TBD logout from sbox, re-auth, is marked as sbox

@mshanemc
Copy link
Contributor

mshanemc commented Aug 2, 2023

QA notes: merged based on demos

@mshanemc mshanemc merged commit cc83f3f into main Aug 2, 2023
31 checks passed
@mshanemc mshanemc deleted the sh/enhance-org-categorization branch August 2, 2023 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants