Skip to content

Swarming: Fetches Credentials with scope & other Minor fixes#5222

Open
IvanBM18 wants to merge 1 commit intomasterfrom
feature/swarming/fix-auth-call
Open

Swarming: Fetches Credentials with scope & other Minor fixes#5222
IvanBM18 wants to merge 1 commit intomasterfrom
feature/swarming/fix-auth-call

Conversation

@IvanBM18
Copy link
Copy Markdown
Collaborator

@IvanBM18 IvanBM18 commented Mar 26, 2026

After testing the remote task gate in dev we were able to discover more things that were missing in the swarming implementation.

Credentials request to retrieve required scopes

Credentials retrieved using credentials.get_default(_SWARMING_SCOPES) didn't have all the requested scopes, even tough we explicitly requested it, this happened because the GCP metadata server ignored the requested scopes and instead returned the VM scopes
image

Note: In red previous workflow, in green new workflow, in gray workflow used for local tests

Minor Fixes

  • Swarming requires the target OS to be capitalized, hence it wont schedule the task to any bot.
  • We added the IS_K8S_ENV as a workaround for this bug: b/495819289

@IvanBM18 IvanBM18 self-assigned this Mar 26, 2026
@IvanBM18 IvanBM18 requested a review from a team as a code owner March 26, 2026 17:54
@IvanBM18 IvanBM18 added the swarming Changes related to the clusterfuzz-swarming integration label Mar 26, 2026
@IvanBM18 IvanBM18 changed the title Swarming: Fetches Credentials with scope, Minor Fixes to request Swarming: Fetches Credentials with scope & other Minor fixes Mar 26, 2026
Job dimensions have more precedence than static dimensions"""
unique_dimensions = {}
unique_dimensions['os'] = job.platform
unique_dimensions['os'] = str(job.platform).capitalize()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is the call to str() actually needed? Isn't job.platform already a string? I suppose we need to capitalize it since Swarming is not accepting the string as is, correct? Is it because it's not a string or because it's not capitalized?

Copy link
Copy Markdown
Collaborator Author

@IvanBM18 IvanBM18 Mar 27, 2026

Choose a reason for hiding this comment

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

I mean the str() casting is not necessary, but i just add it to avoid one more error from displaying in the screen:
image

Worth noting that platform is never empty because is a requirement to add it to the data store

url = f'https://{swarming_server}/prpc/swarming.v2.Tasks/NewTask'
utils.post_url(
url=url, data=json_format.MessageToJson(task_spec), headers=headers)
message_body = json_format.MessageToJson(task_spec)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Isn't this a security concern to log the whole body to the console?
Might want to consider logging it conditionally depending on the environment.

Copy link
Copy Markdown
Collaborator Author

@IvanBM18 IvanBM18 Mar 27, 2026

Choose a reason for hiding this comment

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

I guess not since logs are only readable trough the GCP project, but im not 100% sure of this affirmation, since im not fully aware of who has access to this projects.

@javanlacerda wdyt?

Copy link
Copy Markdown
Collaborator

@jardondiego jardondiego left a comment

Choose a reason for hiding this comment

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

Left a couple questions above. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

swarming Changes related to the clusterfuzz-swarming integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants