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

Restrict allowed characters in connection ids #29836

Closed
2 tasks done
BasPH opened this issue Mar 1, 2023 · 2 comments · Fixed by #31140
Closed
2 tasks done

Restrict allowed characters in connection ids #29836

BasPH opened this issue Mar 1, 2023 · 2 comments · Fixed by #31140
Assignees
Labels
area:core kind:bug This is a clearly a bug

Comments

@BasPH
Copy link
Contributor

BasPH commented Mar 1, 2023

Description

I bumped into a bug where a connection id was suffixed with a whitespace e.g. "myconn ". When referencing the connection id "myconn" (without whitespace), you get a connection not found error.

To avoid such human errors, I suggest restricting the characters allowed for connection ids.

Some suggestions:

  • There's an airflow.utils.helpers.validate_key function for validating the DAG id. Probably a good idea to reuse this.
  • I believe variable ids are also not validated, would be good to check those too.

Use case/motivation

No response

Related issues

No response

Are you willing to submit a PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@BasPH BasPH added kind:feature Feature Requests needs-triage label for new issues that we didn't triage yet labels Mar 1, 2023
@josh-fell josh-fell added area:core and removed needs-triage label for new issues that we didn't triage yet labels Mar 2, 2023
@josh-fell
Copy link
Contributor

Good idea! All yours @BasPH!

@potiuk
Copy link
Member

potiuk commented Mar 2, 2023

Yep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:core kind:bug This is a clearly a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants