Skip to content

Commit

Permalink
[apache#38284] Update Confirmation Logic for Config Changes on Sensit…
Browse files Browse the repository at this point in the history
…ive Environments Like Production (apache#38299)

* - [apache#38284] Update Confirmation Logic for Dag status Changes on Sensitive Environments Like Production

* - Update pause/unpause confirm messages

* - Not version added yet

* - Fix static check

* - Ruff check

* Revert "- Fix static check"

This reverts commit cccfeb7.

* - Fix ruff check
  • Loading branch information
maycuatroi authored Mar 20, 2024
1 parent 7776e91 commit d8f647c
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 1 deletion.
9 changes: 9 additions & 0 deletions airflow/config_templates/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1943,6 +1943,15 @@ webserver:
type: float
example: ~
default: "1.0"
require_confirmation_dag_change:
description: |
Require confirmation when changing a DAG in the web UI. This is to prevent accidental changes
to a DAG that may be running on sensitive environments like production.
When set to True, confirmation dialog will be shown when a user tries to Pause/Unpause, Trigger a DAG
version_added: ~
type: boolean
example: ~
default: "False"
email:
description: |
Configuration email backend and whether to
Expand Down
4 changes: 4 additions & 0 deletions airflow/www/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,17 @@ def create_app(config=None, testing=False):
flask_app.config["SQLALCHEMY_DATABASE_URI"] = conf.get("database", "SQL_ALCHEMY_CONN")

instance_name = conf.get(section="webserver", key="instance_name", fallback="Airflow")
require_confirmation_dag_change = conf.getboolean(
section="webserver", key="require_confirmation_dag_change", fallback=False
)
instance_name_has_markup = conf.getboolean(
section="webserver", key="instance_name_has_markup", fallback=False
)
if instance_name_has_markup:
instance_name = Markup(instance_name).striptags()

flask_app.config["APP_NAME"] = instance_name
flask_app.config["REQUIRE_CONFIRMATION_DAG_CHANGE"] = require_confirmation_dag_change

url = make_url(flask_app.config["SQLALCHEMY_DATABASE_URI"])
if url.drivername == "sqlite" and url.database and not url.database.startswith("/"):
Expand Down
13 changes: 13 additions & 0 deletions airflow/www/extensions/init_appbuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,19 @@ def app_name(self):
"""
return self.get_app.config["APP_NAME"]

@property
def require_confirmation_dag_change(self):
"""Get the value of the require_confirmation_dag_change configuration.
The logic is:
- return True, in page dag.html, when user trigger/pause the dag from UI.
Once confirmation box will be shown before triggering the dag.
- Default value is False.
:return: Boolean
"""
return self.get_app.config["REQUIRE_CONFIRMATION_DAG_CHANGE"]

@property
def app_theme(self):
"""
Expand Down
10 changes: 10 additions & 0 deletions airflow/www/static/js/dag.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,16 @@ $("#pause_resume").on("change", function onChange() {
const $input = $(this);
const id = $input.data("dag-id");
const isPaused = $input.is(":checked");
const requireConfirmation = $input.data("require-confirmation");
if (requireConfirmation) {
const confirmation = window.confirm(
`Are you sure you want to ${isPaused ? "resume" : "pause"} this DAG?`
);
if (!confirmation) {
$input.prop("checked", !isPaused);
return;
}
}
const url = `${pausedUrl}?is_paused=${isPaused}&dag_id=${encodeURIComponent(
id
)}`;
Expand Down
12 changes: 11 additions & 1 deletion airflow/www/templates/airflow/dag.html
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ <h3 class="pull-left">
{% endif %}
<label class="switch-label{{' disabled' if not can_edit_dag else '' }} js-tooltip" title="{{ switch_tooltip }}">
<input class="switch-input" id="pause_resume" data-dag-id="{{ dag.dag_id }}"
data-require-confirmation="{{ appbuilder.require_confirmation_dag_change }}"
type="checkbox"{{ " checked" if not dag_is_paused else "" }}
{{ " disabled" if not can_edit_dag else "" }}>
<span class="switch" aria-hidden="true"></span>
Expand Down Expand Up @@ -261,7 +262,8 @@ <h4 class="pull-right js-dataset-triggered" style="user-select: none;-moz-user-s
{% else %}
<a href="{{ url_for('Airflow.trigger', dag_id=dag.dag_id, origin=url_for(request.endpoint, dag_id=dag.dag_id, **request.args)) }}"
{% endif %}
onclick="return triggerDag(this, '{{ dag.dag_id }}')" title="Trigger&nbsp;DAG"
onclick="return {{ 'triggerDag' if not appbuilder.require_confirmation_dag_change else 'confirmTriggerDag' }}(this, '{{ dag.dag_id }}')"
title="Trigger&nbsp;DAG"
aria-label="Trigger DAG"
class="btn btn-default btn-icon-only{{ ' disabled' if not dag.can_trigger }} trigger-dropdown-btn">
<span class="material-icons" aria-hidden="true">play_arrow</span>
Expand Down Expand Up @@ -301,5 +303,13 @@ <h4 class="pull-right js-dataset-triggered" style="user-select: none;-moz-user-s
postAsForm(link.href, {});
return false;
}

function confirmTriggerDag(link, dagId) {
if (confirm(`Are you sure you want to trigger '${dagId}' now?`)) {
postAsForm(link.href, {});
}
return false;
}

</script>
{% endblock %}

0 comments on commit d8f647c

Please sign in to comment.