8000 Add ftp passive ports by nrx-ops · Pull Request #60 · sagikazarmark/helm-charts · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add ftp passive ports #60

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

nrx-ops
Copy link
@nrx-ops nrx-ops commented Apr 16, 2021

No description provided.

@sagikazarmark
Copy link
Owner

Hi @nrx-ops !

Thanks for the contribution! I have a couple reservations/questions about the implementation though:

  • Configuration does not necessarily come from the config value, it can be mounted as a secret or configmap. Therefore I think it would be better to make this a separate config in the values file which makes it somewhat redundant, but makes the solution more universal for other config sourcing options.
  • You added support for passive ports in the main service, but the additional services will also need to be changed.
  • How should we handle node ports? If someone wishes to use node ports, the assigned ports will be random.

I also wonder how much of an edge case is this. Is this something this chart should support, or could it be a section in the documentation (ie. this is how you can create a service for passive ports). I'm not saying no, but I have more questions than answers at the moment.

What do you think?

@nrx-ops
Copy link
Author
nrx-ops commented Apr 16, 2021

Hi,
Thanks for the review. You made an awesome work with this chart. I not very familiar with GH process, this P/R is in early stage.

  • I really wonder about the way to specify pasv port range from .Value or from config. I had some issues on other charts with duplicate config. I though using .Values.config was a better idea, but I missed secret/cm config. I can change it with a new default value under ftpd. This will avoid some dirty lines.

  • OK, I will add it

  • I have no good idea, for this case. Maybe we could use a startPort and increase for each pasv-port, I will see that and propose you a solution, the other solution is to make incompatible nodePorts and pasv-ports.

I think pasv port should be supported by this chart. sftpgo supports FTP and ftp need pasv-ports to work.

@sagikazarmark
Copy link
Owner

I really wonder about the way to specify pasv port range from .Value or from config. I had some issues on other charts with duplicate config.

The alternative is to make the port range part of the values file and configure it via environment variables. See deployment.yaml for examples.

@vincenthodicq
Copy link

Hi @nrx-ops

did you make it work with FTP with your changes? I tried but I still got some errors on the configuration part of sftpgo.

Regards

@nrx-ops
Copy link
Author
nrx-ops commented Jul 19, 2021

Hi @vincenthodicq,
I make it work on ovh cloud and GCP.
What kind of error do you have? What cloud provider do you use?

@vincenthodicq
Copy link

I deployed it on Azure AKS. I had errors when trying to connect with FTP, i got errors with Passive connection and unable to access any of the passive port.

What type of service did you use to install sftpgo? did you use ingress?

Copy link
Author
nrx-ops commented Jul 19, 2021

OK,
I did not use Ingress, only the loadbalancer service.
On gcp I had to fix firewall rules to make it works.

@vincenthodicq
Copy link

@nrx-ops can you share an example of the values your override for this configuration? (without anything sensitive or protected)

@vincenthodicq
Copy link

I am looking in particular to see if you used force_passive_ip for ftpd configuration. If yes, did you use load balancerIP?

And did you use specific externalTrafficPolicy for your service?

Thanks in advance

@nrx-ops
Copy link
Author
nrx-ops commented Jul 19, 2021
replicaCount: 1
podSecurityContext:
  fsGroup: 1000
sftpd:
  enabled: true
ftpd:
  enabled: true
webdavd:
  enabled: false
config:
  defender:
    enabled: true
  sftpd:
    host_keys:
      - /var/lib/sftpgo/fingerprints/id_rsa
      - /var/lib/sftpgo/fingerprints/id_ecdsa
      - /var/lib/sftpgo/fingerprints/id_ed25519
    kex_algorithms:
      - diffie-hellman-group1-sha1
      - diffie-hellman-group14-sha1
      - ecdh-sha2-nistp256
      - ecdh-sha2-nistp384
      - ecdh-sha2-nistp521
      - curve25519-sha256@libssh.org
    ciphers:
      - aes128-ctr,
      - aes192-ctr
      - aes256-ctr
      - aes128-gcm@openssh.com
      - chacha20Poly1305ID
      - arcfour256
      - arcfour128
      - arcfour
      - aes128cbcID
      - tripledescbcID
  ftpd:
    bindings:
      port: 2021
    passive_port_range:
      start: 20000
      end: 20100
    enable_site: false
  data_provider:
    driver: postgresql
    name: sftpgo
    host: postgresql-sftp
    port: 5432
    username: postgres
    password: [REDACTED]
    prefer_database_credentials: true


service:
  type: LoadBalancer
  externalTrafficPolicy: Local
  loadBalancerIP: [REDACTED]
  annotations: {}
  ports:
    ftpPasv:
      ports:
        start: 20000
        end: 20100
      nodeports:
        start: 30000

volumes:
  - name: cm-fingerprint-sftp
    configMap:
      name: cm-fingerprint-sftp
volumeMounts:
  - mountPath: /var/lib/sftpgo/fingerprints
    name: cm-fingerprint-sftp

@vincenthodicq
Copy link

Thanks. It helps and i am now able to connect with FTP. Still having an issue with FTPS (default port being 990 and not opened / configured in the chart)

@nrx-ops
Copy link
Author
nrx-ops commented Jul 20, 2021

In this case, I think that .Values.service.ports.ftp.port must be set on 990.
Or you should use additionnal service

@OlavH96
Copy link
OlavH96 commented Nov 9, 2023

Any update on this PR? Without exposing passive ports the ftp portion of this helm chart is unusable

@dbt-lucka
Copy link
dbt-lucka commented Nov 14, 2023

It's funny that someone re-rerequested this PR 5 days ago, because I am basically blocked too.
Is there anything I could support in getting this PR merged?
The FTP service is basically unusable or I have to create another Kubernetes service which would probably get its own loadbalancer created 🤔

Edit: You can use Kustomize with helm post-renderers to get the additional port exposures in.
In case you're using Flux it has OOTB support for post-renderer execution.
This works:
https://fluxcd.io/flux/components/helm/helmreleases/#post-renderers

@gioppoluca
Copy link

+1 to this

@BabisK
Copy link
BabisK commented Jan 28, 2024

I agree, consider merging this

@eduardodbr
Copy link

+1 to this

@Dunge
Copy link
Dunge commented Jan 31, 2025

I'm also hitting this issue I believe.
I got sftp working and ftp in active mode, but passive mode doesn't connect.

So I need to manually get the chart locally with those mods to get the passive ports available on the service, then route public ports to it?

edit: Yeah managed to get it working with this PR applied locally. Sad that this was submitted 4 years ago and still not merged.

@sagikazarmark
Copy link
Owner

The chart is being migrated to https://github.com/sftpgo/helm-chart

Please submit PRs there. Thanks

(I'll leave this open for now because there was recent activity. If someone is willing to pick this up, I may be able to get it reviewed, although I don't use this particular feature)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants
0