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.

Sign up for GitHub

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

Jump to bottom

Fix sftp users ssh resource, add custom domain tag and more improvments #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix sftp users ssh resource, add custom domain tag and more improvments #42

wants to merge 1 commit into from

Conversation

Hax7
Copy link

@Hax7 Hax7 commented Feb 15, 2024

what

  • Fixed IAM policies and roles names to have user name because fixed names will prevent policy/role creation since it will be exist
  • Fixed users ssh resource to have for_each and point to the correct public key and user name
  • Add setting custom domain for SFTP server using transfer:customHostname tag
  • deprecate var.enable_sftp in favour of var.enabled

why

  • General improvements and cleaner usage

themaniskshah
themaniskshah approved these changes Feb 21, 2024
View reviewed changes
Copy link
Member

@themaniskshah themaniskshah left a comment

Choose a reason for hiding this comment

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

LGTM!

vaibhav7797
vaibhav7797 approved these changes Feb 21, 2024
View reviewed changes
Copy link
Member

@vaibhav7797 vaibhav7797 left a comment

Choose a reason for hiding this comment

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

LGTM!

@theprashantyadav theprashantyadav requested review from theprashantyadav and removed request for theprashantyadav February 21, 2024 08:42
@Hax7
Copy link
Author

Hax7 commented Feb 21, 2024

I think I still need to update the examples and add details about how to use directory mapping for restricted home directory, it's not clear in the docs and i had to dig in the code for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vaibhav7797 vaibhav7797 vaibhav7797 approved these changes

@themaniskshah themaniskshah themaniskshah approved these changes

@anmolnagpal anmolnagpal Awaiting requested review from anmolnagpal anmolnagpal is a code owner

@clouddrove-ci clouddrove-ci Awaiting requested review from clouddrove-ci clouddrove-ci is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Hax7 @themaniskshah @vaibhav7797