Paths

Table of Contentst

Differential D14368

Fix an issue with incorrect authorization handling in Working Copy build steps
ClosedPublic
Actions

Authored by epriestley on Oct 30 2015, 3:01 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Wed, Jan 22, 12:35 AM

	Unknown Object (File)
	Fri, Jan 17, 10:41 PM

	Unknown Object (File)
	Fri, Jan 17, 8:03 PM

	Unknown Object (File)
	Tue, Dec 31, 1:03 AM

	Unknown Object (File)
	Dec 18 2024, 4:36 AM

	Unknown Object (File)
	Nov 30 2024, 2:27 AM

	Unknown Object (File)
	Nov 30 2024, 2:27 AM

	Unknown Object (File)
	Nov 30 2024, 2:27 AM

Subscribers

tycho.tatitscheff

Details

Reviewers

tycho.tatitscheff
chad

Maniphest Tasks

T9669: Harbormaster working copy build step shouldn't crash when user don't set any also clone repository

Commits

Restricted Diffusion Commit
rPf48a8337048d: Fix an issue with incorrect authorization handling in Working Copy build steps

Summary

Fixes T9669. Two issues:

We were using repositoryPHIDs instead of blueprintPHIDs for the list of allowed blueprints. Use the correct value.
We weren't enforcing allowedBlueprintPHIDs fully correctly. We did require an authorization, so the net effect was correct in nearly all cases, but we could have selected from too large a pool in the case where the application itself was doing the authorization (e.g., from the command line).

Test Plan

Ran a build through Drydock/Harbormaster locally.

Diff Detail

Repository

Branch

alloclease

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 8497
Build 9794: Run Core Tests
Build 9793: arc lint + arc unit

Event Timeline

epriestley updated this revision to Diff 34689.Oct 30 2015, 3:01 PM

epriestley retitled this revision from to Fix an issue with incorrect authorization handling in Working Copy build steps.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: chad.

epriestley added a task: T9669: Harbormaster working copy build step shouldn't crash when user don't set any also clone repository.

epriestley mentioned this in T9671: Harbormaster builds and Drydock leases are stuck.Oct 30 2015, 3:02 PM

Proper withPHIDs() call.

epriestley added inline comments.Oct 30 2015, 3:04 PM

src/applications/drydock/worker/DrydockLeaseUpdateWorker.php
320	This line is the important one we were missing -- we didn't actually do anything with `$blueprint_phids` before.
331	...although it didn't really matter much, because we almost always enforced an effectively identical check here anyway.
351	...but could have gone through this branch without enforcing the check, causing a CLI allocation to end up on a surprising blueprint.

Does this also add the buildable repository asdefault repository cloned ?

Does this also add the buildable repository as default repository cloned ?

The repository for the buildable is already always cloned.

src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php
173–201	Here's where we explicitly configure the lease to clone the buildable's repository, no matter what `Also Clone` is set to.

chad accepted this revision.Oct 30 2015, 3:53 PM

chad edited edge metadata.

This revision is now accepted and ready to land.Oct 30 2015, 3:53 PM

tycho.tatitscheff added inline comments.Oct 30 2015, 3:56 PM

src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php
173–201	That was what I though. I wasn't sure about it though. Thanks for confirming.

tycho.tatitscheff accepted this revision.Oct 30 2015, 3:56 PM

tycho.tatitscheff edited edge metadata.

Closed by commit rPf48a8337048d: Fix an issue with incorrect authorization handling in Working Copy build steps (authored by epriestley, committed by epriestley). · Explain WhyOct 30 2015, 4:02 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

src/

applications/

drydock/

worker/

DrydockLeaseUpdateWorker.php

11 lines

harbormaster/

step/

HarbormasterLeaseWorkingCopyBuildStepImplementation.php

5 lines

Diff 34690

src/applications/drydock/worker/DrydockLeaseUpdateWorker.php

Loading...

src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php

Loading...