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.

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

🍀 Proposal: Cloudflare IP List Monitor #560

Closed
IronCore864 opened this issue May 20, 2022 · 14 comments · Fixed by #648
Closed

🍀 Proposal: Cloudflare IP List Monitor #560

IronCore864 opened this issue May 20, 2022 · 14 comments · Fixed by #648
Assignees
Labels
enhancement New feature or request

Comments

@IronCore864
Copy link
Member

IronCore864 commented May 20, 2022

What would you like to add? Why is this needed?

Background

We use S3 to store published plugins from v0.6.0. And to speed things up, we have Cloudflare in front of the S3 bucket. The thing is, we grant access to S3 by IP addresses.

Cloudflare publishes its IP range here: https://www.cloudflare.com/ips/. It's also available from a text file: https://www.cloudflare.com/ips-v4.

If Cloudflare IP changes, we won't be able to dtm init anymore because S3 doesn't know if Cloudflare changes its IP or not and is still using the old list.

IP Change Monitoring

It would be nice if we can have a daily cronjob that checks if the IP list of Cloudflare has changed or not.

If yes, send a notification (email/feishu/slack) to DevStream PMC.

Functional Requirements

The script can be a simple python one, or a simple go app. We can even utilize github actions or something to that effect. The goal is to have a service like "hascloudflarechangedip.com" or something.

Non-Functional Requirements

Maybe figure out a way to ensure 24 hours SLA.

I.E., at least run the job once per day so that we can be notified within 24 hours (maximum) after IP changed.

@IronCore864 IronCore864 changed the title 🍀 Proposal: CloudFlare 🍀 Proposal: Cloudflare IP List Monitor May 20, 2022
@IronCore864 IronCore864 added the enhancement New feature or request label May 20, 2022
@iyear
Copy link
Member

iyear commented May 23, 2022

This issue might be a good first issue, as the tool does not require going deep into the devstream to understand the details. If someone wants to try it out they can give it a go first.

If the need is urgent at the moment, I can develop it.

This should require a new repo to place the code.

My solution is outlined as follows:

The above libraries are very lightweight, so they are not overly bloated.

If we intend to develop a cross-platform widget, my advice is not to rely on OS services, cron and daemon should be handled by the program.

@algobot76
Copy link
Contributor

algobot76 commented May 23, 2022

I have a simpler solution in minde: everything can be done using a bash script.

  • Store IP addresses in a simple plain text.
  • Read the text file using read.
  • Send API requests using curl.
  • Schedule the task using crontab.

@iyear
Copy link
Member

iyear commented May 23, 2022

I have a simpler solution in minde: everything can be done using a bash script.

  • Store IP addresses in a simple plain text.
  • Read the text file using read.
  • Send API requests using curl.
  • Schedule the task using crontab.

Yes, this is a much simpler solution. I started out with the idea of open sourcing as an widget, providing docker images and cross-platform binaries to make it possible to run everywhere and without relying on a specific OS.

That way, others could start it up immediately if they needed to use it.

How to do this depends on what the requirement ends up being.

@KeHaohaoke
Copy link
Contributor

This issue might be a good first issue, as the tool does not require going deep into the devstream to understand the details. If someone wants to try it out they can give it a go first.

If the need is urgent at the moment, I can develop it.

This should require a new repo to place the code.

My solution is outlined as follows:

Lark: I suggest not to use sdk, just use Feishu official website's development guide and use package resty to do it. Using lark's official guide can facilitate iteration, and others' libraries may not follow up and modify.

@iyear
Copy link
Member

iyear commented May 23, 2022

@KeHaohaoke Okay, I'll do that. Thanks for the help and reply!

@IronCore864 IronCore864 added the good first issue Good for newcomers label May 24, 2022
@IronCore864
Copy link
Member Author

Both designs are great. Thanks, @iyear, and @algobot76.

I think we have two choices here:

  1. Go with @algobot76's simple design, run it somewhere in a VM with corn. This way, it will only be used by DevStream.
  2. Go with @iyear's design, run it as a pod in a K8s cluster, maybe even build a frontend for it, as simple as "hascloudflareipchanged.io", and it returns a simple JSON. We can have another app calling this API periodically, and if it has changed, send a message to Feishu. That is to say, create a "has cloudflare changed its IP" service, and create a client app that consumes that service and notifies.

I personally wouldn't future-proof too much, so my vote is on the first solution.

@daniel-hutao @KeHaohaoke what do you think? Need you guys' input here.

@daniel-hutao
Copy link
Member

This is a difficult requirement to test, and it's likely that we won't receive an alert message for years after we're done. Maybe after a few months, our group names have changed and we won't receive any alerts. In short, I think this requirement is not necessary to spend too much effort, there is no need to do too fancy. Just a simple script that can output useful logs after running. As for this script, maybe we can use actions to run it, and when something goes wrong, actions will report the error. The simpler, the better.

@daniel-hutao
Copy link
Member

Also the script only needs to be put into the hack/, and there is no need to consider a separate repo, that repo, if created, will soon be forgotten.

@KeHaohaoke
Copy link
Contributor

KeHaohaoke commented May 24, 2022

Both designs are great. Thanks, @iyear, and @algobot76.

I think we have two choices here:

  1. Go with @algobot76's simple design, run it somewhere in a VM with corn. This way, it will only be used by DevStream.

@daniel-hutao @KeHaohaoke what do you think? Need you guys' input here.

Maybe it's a typo. It's cron. @IronCore864

@KeHaohaoke
Copy link
Contributor

Using scripts and cron is a lightweight scheme. Easy to implement.

@iyear
Copy link
Member

iyear commented May 25, 2022

So I will write a go file as a simple script and put it in the /hack dir, is that ok?

Or give it to a new contributor to complete? This issue has been a good fisrt issue.

@iyear
Copy link
Member

iyear commented May 30, 2022

Looks like this is a good first issue, please unassign me to avoid misunderstandings, thanks!

Or do I complete the issue?

@IronCore864 IronCore864 removed the good first issue Good for newcomers label Jun 1, 2022
@IronCore864
Copy link
Member Author

@iyear sorry for the late reply.

Since this is an advanced feature (a standalone script/app,) and you are already a contributor, I removed the good first issue label. I hope you won't mind.

As we communicated over our Slack channel, let's proceed with a simple design without too many extra features.

In the future, we might consider running it as a pod or a job in K8s and add Slack notification feature; but for now, we do not need to have it at the moment.

@IronCore864
Copy link
Member Author

@iyear sorry for the late reply.

Since this is an advanced feature (a standalone script/app,) and you are already a contributor, I removed the good first issue label. I hope you won't mind.

As we communicated over our Slack channel, let's proceed with a simple design without too many extra features.

In the future, we might consider running it as a pod or a job in K8s and add a Slack notification feature; but for now, we do not need to have it at the moment.

@daniel-hutao please also have a look at our decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants