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

gitrepo: set conditions in gitCheckout #741

Merged
merged 1 commit into from
May 27, 2022
Merged

Conversation

aryan9600
Copy link
Member

@aryan9600 aryan9600 commented May 27, 2022

Signed-off-by: Sanskar Jaiswal [email protected]

Refer: #727 (comment)

@darkowlzz
Copy link
Contributor

darkowlzz commented May 27, 2022

Looks like we have more test flakes https://github.com/fluxcd/source-controller/runs/6623628981?check_suite_focus=true#step:5:446.
Should be fixable similar to this fix #730

@@ -744,9 +745,10 @@ func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context,
authOpts.TransportOptionsURL = fmt.Sprintf("ssh://%s/%s/%d", obj.Name, obj.UID, obj.Generation)
} else {
e := &serror.Stalling{
Err: fmt.Errorf("git repository URL has invalid transport type: '%s'", obj.Spec.URL),
Err: fmt.Errorf("git repository URL '%s' has invalid transport type, expected to be one of http, https, ssh", obj.Spec.URL),
Copy link
Contributor

@darkowlzz darkowlzz May 27, 2022

Choose a reason for hiding this comment

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

Maybe we should say "supported protocols", instead of expected?
Thinking in terms of what the user would see.

Copy link
Member

Choose a reason for hiding this comment

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

Currently the CLI does something similar:

✗ git URL scheme 'stl' not supported, can be: ssh, http and https

Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

LGTM!
The messaging recommendation can be changed if it makes it any better.

@pjbgf pjbgf added the area/git Git related issues and pull requests label May 27, 2022
@pjbgf pjbgf added this to the GA milestone May 27, 2022
Copy link
Member

@pjbgf pjbgf left a comment

Choose a reason for hiding this comment

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

LGTM

@pjbgf pjbgf merged commit 7953d0e into fluxcd:main May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants