Re: [PATCH v4 0/8] Auto-configure advertised remotes via URL allowlist

[Toon Claes <toon@iotcl.com>]

Christian Couder <christian.couder@gmail.com> writes:

>> But I previously mentioned I felt the naming of 'acceptFromServer' and
>> 'acceptFromServerUrl' are a bit confusing. So I'm wondering whether we
>> can consider another proposal:
>>
>> What if 'acceptFromServer' would configure if 'acceptFromServerUrl'
>> should be used? I mean, imagine we put this in the config:
>>
>>     [promisor]
>>         acceptFromServer = Match
>>         acceptFromServerUrl = https://my-org.com/*
>>
>> (we can still argue over naming, but to get the idea)
>>
>> So the value "Match" for 'acceptFromServer' would inform Git to use
>> 'acceptFromServerUrl'. This way precedence isn't a concern no more,
>> because every value for 'acceptFromServer' is mutually exclusive.
>
> In this case I would prefer to remove 'acceptFromServerUrl' entirely
> and to make acceptFromServer accept values like:
>
>     match:https://my-org.com/*
>
> By the way "match" might not be the best term. Maybe something like
> "auto-configure" would be better.

I think that's too complicated. Let's not do that.

>> This has one downside though, you can no longer combine
>> acceptFromServer=KnownUrl with a 'acceptFromServerUrl'. So URLs
>> advertised by the server can no longer fall-through to
>> 'acceptFromServer' if they don't match 'acceptFromServerUrl'. You can
>> argue whether that's a good thing or not.
>
> I think it's a good thing to have this fall-through. It allows setting
> up things like this:
>
> In the global config:
>
> [promisor]
>         acceptFromServerUrl = https://my-org.com/*
>
> In the config of only a few repo that need it:
>
> [promisor]
>         acceptFromServer = knownUrl
>
> This way remotes from my-org.com are accepted in all the repos, while
> other remotes are accepted only if their name and URLs have already
> been configured in the repos that need them.
>
> This allows relatively lenient security for internal repos and more
> strict security for external ones, and I suspect that many users will
> want something like that.
>
> What you suggest doesn't allow that. It could force users to choose
> for each repo between either URL based allowlist or local
> configuration of every remote.

Well yes, that's why I mentioned:

> You can argue whether that's a good thing or not.

If it's intentional and as you mention there's a valid use-case for
this, then I agree with your approach in this series.

> Also I think it's easier to explain that 'acceptFromServerUrl' is a
> different mechanism (that allows auto-configuration, contrary to
> 'acceptFromServer') if these two variables are independent.

True, although naming-wise it doesn't feel like that. But I no longer
gonna keep picking on that, so ignore this comment please. :-)

>> What do you think? If you disagree, I'm fine with the current approach
>> and I think this version looks good.
>
> Thanks for your review and for being fine with the current approach if
> I disagree.

Thanks for explaining, I still agree moving on like this.

-- 
Cheers,
Toon