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

Create auth manager documentation #36211

Merged
merged 20 commits into from
Dec 19, 2023
Merged

Conversation

vincbeck
Copy link
Contributor

@vincbeck vincbeck commented Dec 13, 2023

Create some documentation about the concept of auth manager. Explain as well how to create a new auth manager. Add documentation as well for the FAB auth manager. I also moved some pages and sections from Airflow documentation to Fab provider documentation.

Resolves #32211


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

specific language governing permissions and limitations
under the License.

API Authentication
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section has been moved from security/api.rst

specific language governing permissions and limitations
under the License.

Webserver authentication
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section has been moved from security/webserver.rst

docs/apache-airflow-providers-fab/auth-manager/index.rst Outdated Show resolved Hide resolved
docs/apache-airflow-providers-fab/auth-manager/index.rst Outdated Show resolved Hide resolved
docs/apache-airflow/security/api.rst Outdated Show resolved Hide resolved
docs/apache-airflow/security/webserver.rst Outdated Show resolved Hide resolved
docs/apache-airflow/core-concepts/auth-manager.rst Outdated Show resolved Hide resolved
docs/apache-airflow/core-concepts/auth-manager.rst Outdated Show resolved Hide resolved
docs/apache-airflow/core-concepts/auth-manager.rst Outdated Show resolved Hide resolved
docs/apache-airflow/core-concepts/auth-manager.rst Outdated Show resolved Hide resolved
@potiuk
Copy link
Member

potiuk commented Dec 14, 2023

Comment/ Proposal. Why don't you prepare one/two diagrams, similar to the ones that you had in the AIP. I recently integrated the nice Python "Diagram as a Code" tool into our pre-commit workflows - and it should be very simple to add new diagrams (I plan to have it more distributed, in a near future and have separate python script for each diagram - next to the diagram it creates, but for now it would be easy to add one more diagram to generate to this pre-commit:

https://github.com/apache/airflow/blob/main/scripts/ci/pre_commit/pre_commit_generate_airflow_diagrams.py

It's super-easy to create diagrams in it -very similar concept as Airflow DAGs in fact :)

@vincbeck
Copy link
Contributor Author

Comment/ Proposal. Why don't you prepare one/two diagrams, similar to the ones that you had in the AIP. I recently integrated the nice Python "Diagram as a Code" tool into our pre-commit workflows - and it should be very simple to add new diagrams (I plan to have it more distributed, in a near future and have separate python script for each diagram - next to the diagram it creates, but for now it would be easy to add one more diagram to generate to this pre-commit:

https://github.com/apache/airflow/blob/main/scripts/ci/pre_commit/pre_commit_generate_airflow_diagrams.py

It's super-easy to create diagrams in it -very similar concept as Airflow DAGs in fact :)

Sounds exciting! Let me try that

@potiuk
Copy link
Member

potiuk commented Dec 14, 2023

Sounds exciting! Let me try that

Indeed, I love how they implemented it and it's really nice to iterate on diagram, it's super-fast- just change the code, run the pre-commit and the image gets regenerated :). You could actually make auto-action on save with it to get it nearly-live-update.

@potiuk
Copy link
Member

potiuk commented Dec 14, 2023

BTW. You can run the pre-comit script "as-is" - as long as you have diagrams installed in your venv it works as regular Python script

@vincbeck
Copy link
Contributor Author

In the meantime, do you know why it is complaining about sphinx.errors.SphinxWarning: /opt/airflow/docs/apache-airflow/security/api.rst:24:unknown document: 'apache-airflow-providers-fab:auth-manager/api-authentication'. This file exists. It does not need to b released first right? (I would not imagine but I am trying to understand why sphinx is complaining about it)

@potiuk
Copy link
Member

potiuk commented Dec 14, 2023

Sphinx speaking riddles again

I will take a look after my talk (I have a talk at https://osacon.io in 45 minutes so a little stressed ;) .

@vincbeck vincbeck requested a review from ashb as a code owner December 14, 2023 16:59
@vincbeck
Copy link
Contributor Author

Sphinx speaking riddles again

I will take a look after my talk (I have a talk at https://osacon.io in 45 minutes so a little stressed ;) .

Thank you :D! And good luck for your talk

@vincbeck
Copy link
Contributor Author

Creating diagrams is actually super fun! I experienced a bug though. When I run the script as is, everything is fine but when I generate the diagrams through the static checks (breeze static checks), the image python_multiprocess_logo.png that we are using in diagrams is not shown

@vincbeck
Copy link
Contributor Author

Ci is green! I had to revert references to the fab provider because Sphinx does not like referencing in flight documents. I'll create another PR when this one gets merged to essentially revert my last commit and add back these references

@potiuk
Copy link
Member

potiuk commented Dec 19, 2023

Ci is green! I had to revert references to the fab provider because Sphinx does not like referencing in flight documents. I'll create another PR when this one gets merged to essentially revert my last commit and add back these references

Yeah there is a certain weirdness in it. It mainly comes from the fact that we have the documentation built in parallel and it might be that one package docs is still using the one from the remote "inventory" stored in AWS rather than the one from locally built docs. I sometimes also split such changes into two to make sure it passes the build.

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

❤️ it

@potiuk potiuk merged commit 4758185 into apache:main Dec 19, 2023
78 checks passed
@potiuk potiuk added this to the Airflow 2.8.1 milestone Dec 19, 2023
@potiuk
Copy link
Member

potiuk commented Dec 19, 2023

I marked it as 2.8.1 for now @vincbeck -> do you feel that the Auth Manager interface in 2.8 is solid enough to bring the docs update to 2.8.1 and expose to the users? or do you think it should be 2.9.0 ?

@vincbeck vincbeck deleted the vincbeck/auth_manager_doc branch December 19, 2023 15:45
@vincbeck
Copy link
Contributor Author

#36175 updated the interface and I dont think it is in 2.8. To me, this was the last interface modification and since then, the auth manager interface is solid and should not change

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

Successfully merging this pull request may close these issues.

AIP-56 - Documentation
4 participants