drone / proposal

Drone Project Design Documents
13 stars 4 forks source link

Re-add a "recursive" flag to the clone step, defaulted to False. #38

Open salomon-smekecohen opened 3 months ago

salomon-smekecohen commented 3 months ago

How does this work right now

Drone's cloning documentation suggests that for repositories with submodules one should add a separate submodule fetch step:

- name: submodules
  image: alpine/git
  commands:
  - git submodule update --init --recursive

there also exists this plugin which can be used to similar effect by:

  1. Disabling the clone step
  2. Using this as a clone step with the --recursive flag enabled.

What I would like to happen

In the clone configuration, allow recursive: true which should work like https://plugins.drone.io/plugins/git, initializing submodules.

Ok but why

DRONE_NETRC_CLONE_ONLY

this flag (which is recommended) limits drone's git access to the clone step. Making submodule cloning not possible in private/auth-required git scenarios for submodule initialization.

Extra information

I found some old documentation that implies this behavior used to exist.

I also believe drone uses that git plugin already to perform the initial cloning, so hopefully this is not a bear to implement.

See:

Note that Drone uses the git plugin by default for all repositories, without any configuration required.

I see this comment from 2019 but I do not know if this being impossible with the recommended security configuration was considered in the decision to remove this. I believe this can be mitigated by updating the docs to specify the extra submodules step first and only mentioning recursive: true as a backup for specifically this scenario, only supporting https.

What I will do if this is denied to work around

hope for an idea in the harness slack?

I think with my company's current security concerns working around this would be... rough. Id probably need to inject an ssh-key via secrets, use sed to alter the .gitmodules configuration to use a git+ instead of https, and pray the team does not fire me for jank.

Very rude tag

@bradrydzewski Sorry for the tag bradrydzewski, but given you seem to be very directly involved with:

  1. the decision to unsupport this.
  2. the old documentation that supported this.

I wanted to make sure you got right-of-first refusal here.