[chore] Cleanup GitLab source errors #2345

mcastorina · 2024-01-26T19:47:32Z

Description:

Cleanup error handling and creation in GitLab source.

Checklist:

Tests passing (make test-community)?
Lint passing (make lint this requires golangci-lint)?

mcastorina · 2024-01-26T19:48:45Z

pkg/sources/gitlab/gitlab.go

 		// Ensure the urls are valid before adding them to the repo list.
 		_, err := url.Parse(prj.HTTPURLToRepo)
 		if err != nil {
-			fmt.Printf("could not parse url given by project: %s", prj.HTTPURLToRepo)
+			ctx.Logger().Error(err, "could not parse url given by project", "project", prj.HTTPURLToRepo)
+			continue
 		}
 		repos = append(repos, prj.HTTPURLToRepo)


Control flow change here (added the continue)

mcastorina · 2024-01-26T19:49:18Z

pkg/sources/gitlab/gitlab.go

-			defer os.RemoveAll(path)
 			if err != nil {
 				scanErrs.Add(err)
 				return nil
 			}
+			defer os.RemoveAll(path)


Moved this defer to after the error check.

rosecodym

what prompted this?

(also, any idea why it's re-scanning the whole repo for secrets?)

rosecodym · 2024-01-29T19:09:34Z

pkg/sources/gitlab/gitlab.go

@@ -108,7 +109,7 @@ func (s *Source) Init(_ context.Context, name string, jobId sources.JobID, sourc
 		// We may need the password as a token if the user is using an access_token with basic auth.
 		s.token = cred.BasicAuth.Password
 	default:
-		return errors.Errorf("Invalid configuration given for source. Name: %s, Type: %s", name, s.Type())
+		return fmt.Errorf("invalid auth method")


is the information you removed injected farther up the call stack?

hmm.. I'm not entirely sure. I don't think we can actually trigger this error 🤔

i'm sure enterprise can with a malformed config file. might be worth double-checking.

mcastorina · 2024-01-30T07:21:43Z

what prompted this?

I started adding source unit support for gitlab and decided to clean it up (separately) while I was there.

(also, any idea why it's re-scanning the whole repo for secrets?)

Nope, though I think it's been happening for awhile.. it's tracked in THOG-1640

ahrav · 2024-01-30T15:08:55Z

pkg/sources/gitlab/gitlab.go

-var _ sources.Source = (*Source)(nil)
-var _ sources.SourceUnitUnmarshaller = (*Source)(nil)
-var _ sources.Validator = (*Source)(nil)
+var _ interface {


Did we decide to use this version of the compile check? I know there was some convo out of band around errors not being as clear, not sure if that was a completely separate convo or related to this?

that was me asking your question internally 😄

This PR probably prompted the convo - I'll change it back

mcastorina requested a review from a team as a code owner January 26, 2024 19:47

mcastorina commented Jan 26, 2024

View reviewed changes

rosecodym approved these changes Jan 29, 2024

View reviewed changes

ahrav reviewed Jan 30, 2024

View reviewed changes

ahrav approved these changes Jan 30, 2024

View reviewed changes

rosecodym mentioned this pull request Jan 30, 2024

Scan GitHub wikis #2233

Merged

2 tasks

mcastorina added 2 commits February 2, 2024 19:32

[chore] Cleanup GitLab source errors

4864eca

Ungroup compile time interface checks and revert error message

00ee5be

mcastorina force-pushed the chore-gitlab-err-cleanup branch from 2f6c919 to 00ee5be Compare February 3, 2024 03:33

mcastorina merged commit 27b30e6 into main Feb 3, 2024
10 checks passed

mcastorina deleted the chore-gitlab-err-cleanup branch February 3, 2024 04:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[chore] Cleanup GitLab source errors #2345

[chore] Cleanup GitLab source errors #2345

mcastorina commented Jan 26, 2024

mcastorina Jan 26, 2024

mcastorina Jan 26, 2024

rosecodym left a comment

rosecodym Jan 29, 2024

mcastorina Jan 30, 2024

rosecodym Jan 30, 2024

mcastorina commented Jan 30, 2024

ahrav Jan 30, 2024

rosecodym Jan 30, 2024

mcastorina Jan 30, 2024

[chore] Cleanup GitLab source errors #2345

[chore] Cleanup GitLab source errors #2345

Conversation

mcastorina commented Jan 26, 2024

Description:

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rosecodym left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcastorina commented Jan 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment