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

feat(networkfirewall): added loggingconfiguration for networkfirewall #641

Conversation

haarchri
Copy link
Member

@haarchri haarchri commented Apr 1, 2023

Description of your changes

  • reorder externalname.go for networkfirewall
  • added networkfirewall loggingconfiguration

note: There are fields inside logDestination that could be have selector and refs like s3, cloudWatchLogs, KinesisDataFirehose when logDestinationType is set - but its not possible to generate selector and refs with crossplane-runtime one possible issue: crossplane/crossplane-runtime#350

examples:

    - logDestinationConfig:
      - logDestination:
          bucketName: uptest-networkfirewall-logging
          prefix: /example
        logDestinationType: S3
      - logDestinationConfig:
          - logDestination:
              logGroup: "aws-network-firewall/firewall/alerts"
            logDestinationType: CloudWatchLogs
            logType: ALERT
          - logDestination:
              logGroup: "aws-network-firewall/firewall/flows"
            logDestinationType: CloudWatchLogs
            logType: FLOW

Fixes #390

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

create, update, delete

NAME                                       READY   SYNCED   EXTERNAL-NAME              AGE
subnet.ec2.aws.upbound.io/sample-subnet1   True    True     subnet-0e44fbc680011d299   13m

NAME                                READY   SYNCED   EXTERNAL-NAME           AGE
vpc.ec2.aws.upbound.io/sample-vpc   True    True     vpc-0c566cb59ff7fcbb1   13m

NAME                                                 READY   SYNCED   EXTERNAL-NAME   AGE
rulegroup.networkfirewall.aws.upbound.io/rulegroup   True    True     rulegroup       13m

NAME                                                          READY   SYNCED   EXTERNAL-NAME                                                           AGE
loggingconfiguration.networkfirewall.aws.upbound.io/example   True    True     arn:aws:network-firewall:us-west-1:123456789101:firewall/firewallname   13m

NAME                                                           READY   SYNCED   EXTERNAL-NAME    AGE
firewallpolicy.networkfirewall.aws.upbound.io/firewallpolicy   True    True     firewallpolicy   13m

NAME                                                   READY   SYNCED   EXTERNAL-NAME                                                           AGE
firewall.networkfirewall.aws.upbound.io/firewallname   True    True     arn:aws:network-firewall:us-west-1:123456789101:firewall/firewallname   13m

NAME                                                      READY   SYNCED   EXTERNAL-NAME                    AGE
bucket.s3.aws.upbound.io/uptest-networkfirewall-logging   True    True     uptest-networkfirewall-logging   13m
apiVersion: networkfirewall.aws.upbound.io/v1beta1
kind: LoggingConfiguration
metadata:
  annotations:
    crossplane.io/external-create-pending: "2023-04-06T11:13:30+02:00"
    crossplane.io/external-create-succeeded: "2023-04-06T11:13:30+02:00"
    crossplane.io/external-name: arn:aws:network-firewall:us-west-1:123456789101:firewall/firewallname
    meta.upbound.io/example-id: networkfirewall/v1beta1/loggingconfiguration
    upjet.crossplane.io/provider-meta: "null"
  creationTimestamp: "2023-04-06T09:22:35Z"
  finalizers:
  - finalizer.managedresource.crossplane.io
  generation: 1
  labels:
    testing.upbound.io/example-name: example
  name: example
  resourceVersion: "290904"
  uid: f3fb9a8e-dc56-49af-b296-a09a4a1abf54
spec:
  deletionPolicy: Delete
  forProvider:
    firewallArn: arn:aws:network-firewall:us-west-1:123456789101:firewall/firewallname
    firewallArnRef:
      name: firewallname
    firewallArnSelector:
      matchLabels:
        testing.upbound.io/example-name: example
    loggingConfiguration:
    - logDestinationConfig:
      - logDestination:
          bucketName: uptest-networkfirewall-logging
          prefix: /example
        logDestinationType: S3
        logType: FLOW
    region: us-west-1
  providerConfigRef:
    name: default
status:
  atProvider:
    id: arn:aws:network-firewall:us-west-1:123456789101:firewall/firewallname
  conditions:
  - lastTransitionTime: "2023-04-06T09:22:50Z"
    reason: Finished
    status: "True"
    type: AsyncOperation
  - lastTransitionTime: "2023-04-06T09:22:50Z"
    reason: Available
    status: "True"
    type: Ready
  - lastTransitionTime: "2023-04-06T09:22:50Z"
    reason: ReconcileSuccess
    status: "True"
    type: Synced

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/networkfirewall/firewall.yaml"

@sergenyalcin
Copy link
Collaborator

According to uptest job logs, the provisioning step was successful but the deletion step failed. The resource that has a deletion problem is S3 Bucket. I am not sure but, I think the reason can be that LoggingConfiguration resource writes some files to S3 Bucket then because of bucket is not empty, it cannot be deleted.

@haarchri
Copy link
Member Author

haarchri commented Apr 4, 2023

@sergenyalcin yes the problem is that the bucket is not empty - any hint to do a force deletion ?

@sergenyalcin
Copy link
Collaborator

@sergenyalcin yes the problem is that the bucket is not empty - any hint to do a force deletion ?

We may consider adding a deletionPolicy Orphan for S3 Bucket resource. I think, by this way, we can unblock this PR. After the test successfully completed, we can clear this bucket manually.

@haarchri haarchri force-pushed the feature/networkfirewall-loggingconfiguration branch from dc05404 to 8dea0a8 Compare April 6, 2023 09:27
@haarchri
Copy link
Member Author

haarchri commented Apr 6, 2023

/test-examples="examples/networkfirewall/firewall.yaml"

@haarchri
Copy link
Member Author

haarchri commented Apr 6, 2023

@sergenyalcin test is now okay
we tested also import of this resource everything is working - think it's save to merge ;)

Copy link
Collaborator

@sergenyalcin sergenyalcin left a comment

Choose a reason for hiding this comment

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

Thanks @haarchri LGTM

@sergenyalcin sergenyalcin merged commit 7b8a615 into crossplane-contrib:main Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Moving networkfirewall(1), resources to v1beta1 version
2 participants