flux-iac/tofu-controller

Branch planner does plan for all opened PRs in the repo despite on branch

hirenko-v opened this issue · 4 comments

I have such gitrepo

apiVersion: source.toolkit.fluxcd.io/v1
kind: GitRepository
metadata:
  annotations:
  name: flux-system
  namespace: flux-system
spec:
  interval: 1m0s
  ref:
    branch: flux
  secretRef:
    name: flux-system
  timeout: 60s
  url: ssh://git@github.com/myorg/infrastructure.git

and such Terraform

apiVersion: infra.contrib.fluxcd.io/v1alpha2
kind: Terraform
metadata:
  name: example
  namespace: flux-system
spec:
  alwaysCleanupRunnerPod: true
  backendConfig:
    customConfiguration: "# Override"
  destroyResourcesOnDeletion: false
  interval: 1m0s
  path: ./terraform/example
  refreshBeforeApply: false
   serviceAccountName: tf-runner
  sourceRef:
    kind: GitRepository
    name: flux-system
    namespace: flux-system
  storeReadablePlan: human

and branch-planner config:

apiVersion: v1
data:
  resources: |-
  -  namespace: flux-system
  secretName: branch-planner-token
kind: ConfigMap
metadata:
  name: branch-planner
  namespace: flux-system

and seems like it does plan for all open PRs despite on the repo config

  ref:
   branch: flux

It doesn't happen with rc2 but rc3 and rc4 seems to be broken

Thank you for trying out the branch planner.

While @yitsushi could be the best person right now to point out where to fix, but the project is now 100% community driven.
PRs are always welcome from community members like you @hirenko-v

and seems like it does plan for all open PRs despite on the repo config

Branch planner does not filter on target branch, and as I remember, it never did. That's not a good behaviour, but I don't think we had that use-case in mind (PRs can be opened against other branches and people may use it).

If it did not create PRs previously, that was an bug not a feature that time 🤦‍♀️

@yitsushi I think the behaviour is not correct, it should filter PRs based on branch (ref) in GitRepository, it makes no sense if I have Terraform resource that use main branch from GitRepository to do plan for PRs opened to staging branch.
Also good idea is to have changeset filter to make plan on PRs that include changes in directory configured in Terraform CR

That's not a good behaviour, but I don't think we had that use-case in mind (PRs can be opened against other branches and people may use it). (#1263 (comment))

I mean yes... The behaviour is not good. That's what I wrote, but that's the behaviour it has now. Yes I agree it's a missing feature, as we didn't think of this logic when we built it.

I know the "bug not a feature that time" can be confusing, but that was about you mentioned previously it did not do that, and I'm sure it did as we added branch filtering (and we didn't remove it either), therefore if it did not do that in previous releases, it was a bug somewhere else as it was the "how the code works" logic on branch planner.

Also good idea is to have changeset filter to make plan on PRs that include changes in directory configured in Terraform CR

We have that with enablePathScope, but it's not enabled by default. relevant code