nautobot / nautobot-app-golden-config

Golden Configuration App for Nautobot.
https://docs.nautobot.com/projects/golden-config/en/latest/
Other
98 stars 56 forks source link

Computed field causes tables.ConfigComplianceTable to return KeyError and crash the view #469

Open netopsengineer opened 1 year ago

netopsengineer commented 1 year ago

Environment

Steps to Reproduce

  1. Navigate to yourdomain.com/plugins/golden-config/config-compliance/
  2. Click on Configure and Select/De-Select a column representing any compliance feature to show/hide from table view
  3. Error instantly occurs
  4. Clear the error by navigating to User > Profile > Preferences, remove tables.ConfigComplianceTable.columns settings
  5. Navigate back to yourdomain.com/plugins/golden-config/config-compliance/ and the site is working again

Expected Behavior

Show/Hide Compliance Features not relevant to the devices in the view.

Observed Behavior

<class 'KeyError'>

'cpf_compliance_change'

Python version: 3.9.16
Nautobot version: 1.5.15

The computed field referenced is the only computed field that is currently configured in our deployment:

content_type: "nautobot_golden_config | config compliance"
label: "Compliance Change"
slug: "compliance_change"
description: "Timestamp for last time compliance changed for a feature"
template: "{{ obj | get_change_log }}"
fallback_value: ""
weight: 100
move_to_advanced_tab: Null

tagging @jdrew82 @bile0026 for awareness

jeffkala commented 1 year ago

Was able to duplicate this issue in demo.nautobot.com Looks like in ConfigComplianceTable something a bit odd with the Super call not actually taking into account CF and CPFs like I would anticipate.

Manually adding the for loop in the ConfigComplianceTable as shown below fixes the problem but need to identify the real cause.

# ConfigCompliance
class ConfigComplianceTable(BaseTable):
    """Table for rendering a listing of Device entries and their associated ConfigCompliance record status."""

    pk = ToggleColumn(accessor=A("device"))
    device = TemplateColumn(
        template_code="""<a href="{% url 'plugins:nautobot_golden_config:configcompliance_devicedetail' pk=record.device  %}" <strong>{{ record.device__name }}</strong></a> """
    )

    def __init__(self, *args, **kwargs):
        """Override default values to dynamically add columns."""
        # Used ConfigCompliance.objects on purpose, vs queryset (set in args[0]), as there were issues with that as
        # well as not as expected from user standpoint (e.g. not always the same values on columns depending on
        # filtering)
        features = list(
            models.ConfigCompliance.objects.order_by("rule__feature__slug")
            .values_list("rule__feature__slug", flat=True)
            .distinct()
        )
        obj_type = ContentType.objects.get_for_model(self.Meta.model)
        for cpf in ComputedField.objects.filter(content_type=obj_type):
            self.base_columns[f"cpf_{cpf.slug}"] = ComputedFieldColumn(cpf)

        extra_columns = [(feature, ComplianceColumn(verbose_name=feature)) for feature in features]
        kwargs["extra_columns"] = extra_columns
        # Nautobot's BaseTable.configurable_columns() only recognizes columns in self.base_columns,
        # so override the class's base_columns to include our additional columns as configurable.
        self.base_columns = copy.deepcopy(self.base_columns)
        for feature, column in extra_columns:
            self.base_columns[feature] = column
        super().__init__(*args, **kwargs)
jeffkala commented 1 year ago

To be clear on this one. Issue occurs with both CF and CPF

jeffkala commented 1 year ago

Calling super() twice in this instance is actually the quick fix. The initial super properly configures all the base_columns from the parent class. We can then make our required updates for the functionality and then call super again for final work. Need to research if this is a bad practice.

    def __init__(self, *args, **kwargs):
        """Override default values to dynamically add columns."""
        # Used ConfigCompliance.objects on purpose, vs queryset (set in args[0]), as there were issues with that as
        # well as not as expected from user standpoint (e.g. not always the same values on columns depending on
        # filtering)
        super().__init__(*args, **kwargs)
        features = list(
            models.ConfigCompliance.objects.order_by("rule__feature__slug")
            .values_list("rule__feature__slug", flat=True)
            .distinct()
        )
        extra_columns = [(feature, ComplianceColumn(verbose_name=feature)) for feature in features]
        kwargs["extra_columns"] = extra_columns
        # Nautobot's BaseTable.configurable_columns() only recognizes columns in self.base_columns,
        # so override the class's base_columns to include our additional columns as configurable.
        self.base_columns = copy.deepcopy(self.base_columns)
        for feature, column in extra_columns:
            self.base_columns[feature] = column
        super().__init__(*args, **kwargs)