-
Notifications
You must be signed in to change notification settings - Fork 41
feat(pr-detection): Autodetect PR information #2617
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
Conversation
Signed-off-by: Javier Rodriguez <[email protected]>
Signed-off-by: Javier Rodriguez <[email protected]>
Signed-off-by: Javier Rodriguez <[email protected]>
Signed-off-by: Javier Rodriguez <[email protected]>
|
Probably it's been already discussed:
|
| // Log warning but don't fail - will fall back to inline storage | ||
| action.Logger.Warn().Err(err).Msg("failed to get CAS credentials for PR metadata, will store inline") | ||
| } else { | ||
| b := creds.GetResult().GetBackend() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't we have this CAS logic already elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have this logic on attestation push
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see it's also in attestation add, so maybe it can we moved to a common function and reuse it. Something like:
var casBackend = &casclient.CASBackend{Name: "not-set"}
if !action.dryRun && attestationID != "" {
casBackend, err = getCASBackend(ctx, attestationID)
...
}
Signed-off-by: Javier Rodriguez <[email protected]>
Signed-off-by: Javier Rodriguez <[email protected]>
Signed-off-by: Javier Rodriguez <[email protected]>
Signed-off-by: Javier Rodriguez <[email protected]>
Signed-off-by: Javier Rodriguez <[email protected]>
|
|
||
| // getCASBackend tries to get CAS upload credentials and set up a CAS client | ||
| // If strict is true, errors are returned; if false, warnings are logged and nil error is returned | ||
| func getCASBackend(ctx context.Context, client pb.AttestationServiceClient, workflowRunID, casCAPath, casURI string, casConnectionInsecure bool, logger zerolog.Logger, casBackend *casclient.CASBackend) (func() error, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jiparis the refactor
Signed-off-by: Javier Rodriguez <[email protected]>
Signed-off-by: Javier Rodriguez <[email protected]>
Signed-off-by: Javier Rodriguez <[email protected]>
Implemented automatic detection and collection of Pull Request (GitHub) and Merge Request (GitLab) metadata during attestation initialization when running in CI/CD PR/MR contexts.
Key Features
Automatic PR/MR Detection
GITHUB_EVENT_NAMEandGITHUB_EVENT_PATHCI_PIPELINE_SOURCEand merge request environment variablesNew Material Type:
CHAINLOOP_PR_INFOCHAINLOOP_PR_INFOmaterial type to workflow contractsattestation initin PR/MR contextsSchema Generation & Validation
internal/prinfopackage with typed Go structsjsonschemaannotationshttps://schemas.chainloop.dev/prinfo/1.0/pr-info.schema.jsonImplementation Details
pkg/attestation/crafter/prmetadata.go: Detection logic for GitHub/GitLabpkg/attestation/crafter/materials/chainloop_pr_info.go: Material crafter with validationapp/cli/pkg/action/attestation_init.go: Auto-collection during init