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

models.models.AuthProxySettings.tokenreivew_endpoint is misspelled #19

Open
pederhan opened this issue Jan 10, 2023 · 0 comments
Open
Labels
API Spec Errors or inconsistencies in the API spec bug Something isn't working
Milestone

Comments

@pederhan
Copy link
Member

pederhan commented Jan 10, 2023

The name of this field is misspelled in the official Harbor swagger schema.

This misspelling is also reflected in our auto generated models:
https://github.com/pederhan/harborapi/blob/d4e07cec652f39205a707a7bd876bd70e434df9d/harborapi/models/models.py#L748-L751

Proposed solution

The field name should be tokenreview_endpoint, but we can't just rename the field without breaking validation. We should probably add a field alias in case this is fixed upstream, so that validation doesn't break. But even it's fixed in newer versions of Harbor, we can't remove the old one, as it would break validation for users of older versions of Harbor.

While harborapi is still very much unused by the general public, we should take this opportunity to rename the field to tokenreview_endpoint and use tokenreivew_endpoint as the alias, so that attribute access via dot notation uses the correctly spelled name.

class AuthproxySetting(BaseModel):
    # ...
   tokenreview_endpoint: Optional[str] = Field(
        None,
        description="The fully qualified URI of token review endpoint of authproxy, such as 'https://192.168.1.2:8443/tokenreview'",
        alias="tokenreivew_endpoint"
    )
    # ...
    class Config:
        allow_population_by_field_name = True

We need to allow it to be populated by both spellings of the name to ensure forwards and backwards compatibility. Therefore I am 99% sure we need to add allow_population_by_field_name = True to the config.

@pederhan pederhan added bug Something isn't working good first issue Good for newcomers and removed good first issue Good for newcomers labels Jan 10, 2023
@pederhan pederhan changed the title Add field alias for models.models.AuthProxySettings.tokenreivew_endpoint models.models.AuthProxySettings.tokenreivew_endpoint is misspelled Jan 10, 2023
@pederhan pederhan added this to the Fix API Spec milestone Mar 1, 2023
@pederhan pederhan added the API Spec Errors or inconsistencies in the API spec label Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Spec Errors or inconsistencies in the API spec bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant